Conversation
|
Depends on ipfs/go-ipfs-config#96 |
|
|
||
| Default: `false` | ||
|
|
||
| ### `Peering` |
There was a problem hiding this comment.
include expected behavior / semantics for how this behaves when one of the two peers has it, and when both have it mutually.
peering/peering.go
Outdated
| nextDelay time.Duration | ||
| } | ||
|
|
||
| func (ph *peerHandler) stop() { |
aschmahmann
left a comment
There was a problem hiding this comment.
Seems reasonable to me, I left a few comments/change requests but nothing super dramatic in case you want to just merge this and come back to it later.
| - [`Peering`](#peering) | ||
| - [`Peering.Peers`](#peeringpeers) |
There was a problem hiding this comment.
should any of this behavior going under the experimental label or no?
There was a problem hiding this comment.
I figured it was a minimal enough feature that that wasn't really necessary. I want people to start using this feature. Thoughts?
There was a problem hiding this comment.
But we can revisit this decision once the RC has shipped.
There was a problem hiding this comment.
Seems fine, I wasn't really sure what qualified as experimental vs not which is why I asked 😄
MVP for #6097 This feature will repeatedly reconnect (with a randomized exponential backoff) to peers in a set of "peered" peers. In the future, this should be extended to: 1. Include a CLI for modifying this list at runtime. 2. Include additional options for peers we want to _protect_ but not connect to. 3. Allow configuring timeouts, backoff, etc. 4. Allow groups? Possibly through textile threads. 5. Allow for runtime-only peering rules. 6. Different reconnect policies. But this MVP should be a significant step forward.
Co-authored-by: Will <will.scott@protocol.ai>
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
* better name for timer * cancel context from within stop
* Explain _why_ it exists. * Explain how it can be used.
While preserving some randomness. And add a test.
|
@aschmahmann could you give this a once over post merge? I think it's good enough to go in the RC, but I'd like you to look at my changes. |
aschmahmann
left a comment
There was a problem hiding this comment.
@Stebalien left a small comment on the go test, but LGTM
| return h1.Network().Connectedness(h2.ID()) == network.Connected | ||
| }, 30*time.Second, 100*time.Millisecond) | ||
|
|
||
| require.Len(t, h1.Network().Peers(), 3) |
There was a problem hiding this comment.
I don't know if it's worth doing anything about this, but noting that there's an unlikely race with the connmgr here where we prune in between connecting and doing the size check.
There was a problem hiding this comment.
I set the high water to 100 so that shouldn't happen.
MVP for #6097
This feature will repeatedly reconnect (with a randomized exponential backoff) to peers in a set of "peered" peers.
In the future, this should be extended to:
But this MVP should be a significant step forward.