refactor(multipath): Store Watcher, not Watchable on Connection
#3631
+110
−102
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Currently the
Connectionstores aWatchablefor the open and selected paths. This means, though, that a watcher stream for the open paths will only close once the last connection is dropped, not once the connection is closed. That is bad API and will lead to confusion.In an initial version, I swapped the Watchables for a
n0_watcher::Map<(Direct, Direct)>but this increases the size of theConnectionstruct from ~80 to ~700 bytes, because it would store both the two unmapped values and the mapped final value inline. It is also completely unneeded, because we'd clone out the watcher anyways onConnection::watchandgetorstreamon the watcher would refetch the value.Instead, this now uses n0-computer/n0-watcher#31 to only store a weak pointer to the watchable's shared state without keeping it alive, and then creates the PathInfos on demand from that. The
statsare now fetched while iterating, so if you're e.g. only checking the number of paths, the stats won't be fetched at all.We fetch the paths through upgrading the handle in the
nextmethod, so that we can returnNoneif the connection was dropped in the meantime (the alternative would be a falliblePathInfo::statsmethod)Breaking Changes
Notes & open questions
Change checklist
quic-rpciroh-gossipiroh-blobsdumbpipesendme