node/wire: Always report fetch results to the service
A fetch result was discarded whenever the peer was no longer `Connected` by the time the worker reported, and queued fetches were dropped silently when the peer disconnected before the `Io::Fetch` was processed. In both cases the fetcher's `active` entry for the repo was never cleared. Since that entry is keyed by repo, it blocked the repository from being fetched from any node until the process restarted. - Report the result in `worker_result` even when the peer is no longer connected, instead of returning early - Report a failed fetch when an `Io::Fetch` is dropped for a disconnected peer, so the active entry is cleared - Flip the wire regression test to assert the entry is now cleared on disconnect
This commit is contained in:
parent
1070b777da
commit
7b910476c3
|
|
@ -414,10 +414,10 @@ where
|
||||||
.push_back(Action::Send(fd, frame.encode_to_vec()));
|
.push_back(Action::Send(fd, frame.encode_to_vec()));
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// If the peer disconnected, we'll get here, but we still want to let the service know
|
// If the peer disconnected, we still let the service know about the fetch result.
|
||||||
// about the fetch result, so we don't return here.
|
// Otherwise the fetcher's `active` entry for this repo is never cleared, which blocks
|
||||||
log::debug!(target: "wire", "Peer {nid} is not connected; ignoring fetch result");
|
// the repository from being fetched from any node until the node restarts.
|
||||||
return;
|
log::debug!(target: "wire", "Peer {nid} is not connected; reporting fetch result anyway");
|
||||||
};
|
};
|
||||||
|
|
||||||
// Only call into the service if we initiated this fetch.
|
// Only call into the service if we initiated this fetch.
|
||||||
|
|
@ -1025,9 +1025,18 @@ where
|
||||||
else {
|
else {
|
||||||
// Nb. It's possible that a peer is disconnected while an `Io::Fetch`
|
// Nb. It's possible that a peer is disconnected while an `Io::Fetch`
|
||||||
// is in the service's i/o buffer. Since the service may not purge the
|
// is in the service's i/o buffer. Since the service may not purge the
|
||||||
// buffer on disconnect, we should just ignore i/o actions that don't
|
// buffer on disconnect, we drop fetches that don't have a connected peer.
|
||||||
// have a connected peer.
|
// We must still report the failure so the fetcher clears its `active`
|
||||||
|
// entry; otherwise the repository can no longer be fetched from any node.
|
||||||
log::debug!(target: "wire", "Peer {remote} is not connected: dropping fetch");
|
log::debug!(target: "wire", "Peer {remote} is not connected: dropping fetch");
|
||||||
|
self.service.fetched(
|
||||||
|
rid,
|
||||||
|
remote,
|
||||||
|
Err(crate::worker::FetchError::Io(io::Error::new(
|
||||||
|
io::ErrorKind::NotConnected,
|
||||||
|
"peer disconnected before fetch could start",
|
||||||
|
))),
|
||||||
|
);
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
let (stream, channels) = streams.open(
|
let (stream, channels) = streams.open(
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue