feat: gossip improvements#5520
Conversation
Discard buffered gossip on shutdown instead of flushing, restore quit checks in broadcastNow, document the async BroadcastPeers contract, use fixed 100ms jitter, and clean up tests and metrics. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| select { | ||
| case <-ctx.Done(): | ||
| return total - len(peers), ctx.Err() |
There was a problem hiding this comment.
why do you care about the count when context is cancelled? also, why do you care about the count when an error is returned in general (below, shutting down/other send error)
| mu sync.Mutex | ||
| pending map[string]*pendingGossip // addressee bytestring -> buffered peers | ||
| interval time.Duration | ||
| jitter time.Duration |
| e, ok := b.pending[key] | ||
| if !ok { | ||
| var jitter time.Duration | ||
| if b.jitter > 0 { |
There was a problem hiding this comment.
not sure why this is needed...
| type pendingGossip struct { | ||
| addressee swarm.Address | ||
| peers map[string]swarm.Address // peer bytestring -> address (set semantics) | ||
| deadline time.Time |
There was a problem hiding this comment.
i wonder whether there's a benefit of keeping a deadline per peer. usually, write coalescing is simple enough:
- have an interval fire at a constant rate
- if new records arrived by interval fire
- when new entries arrived, optionally, postpone the sending the sending until the next interval firing (and so also you could extend up to a set upper bound, so that entries don't keep collecting forever but also guarantee that information goes out still relatively quickly)
- send all the pending sends
also: usually, when a peer arrives - we gossip that peer to all peers (full nodes) and gossip to that peer all of our connected peers.
this in turn means that sending is almost always involving all connected peers. which in turn also means that the timestamps on the individual pendingGossip entries would be almost identical (making the field even more so redundant)
| } | ||
|
|
||
| if len(e.peers) >= b.maxBatch { | ||
| delete(b.pending, key) |
There was a problem hiding this comment.
this is really confusing - we first add to the item on the map just to also maybe potentially return it directly? not really add...
2 participants
Checklist
Description
Adds write coalescing for hive outbound gossip. Single-peer BroadcastPeers calls are buffered per addressee and flushed as one batched message after ~1s (configurable via GossipCoalesceInterval), or immediately when the buffer reaches maxBatchSize (30). Calls with 2+ peers are sent without coalescing
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
#5490
Screenshots (if appropriate):
AI Disclosure