feat(swip25): pull syncing optimization #5194
Conversation
nugaon
changed the title
martinconic
changed the title
| } | ||
| p.logger.Debug("radius decrease", "old_radius", prevRadius, "new_radius", newRadius) | ||
| } | ||
| changed := newRadius != prevRadius |
There was a problem hiding this comment.
can you rename please this to something like radiusChanged or rChanged. It would be easier to figure out in the code what is this boolean all about.
There was a problem hiding this comment.
it is related to that the parameters for the pulling have changed that partially includes the radius change but also when a new peer is conencted to the node and that is a neighbor or a neighbor node is disconnected.
| if err != nil { | ||
| // cannot go further in the binary representation, override leaf | ||
| if t.V != nil && !bytes.Equal(t.K, key) { | ||
| panic(errShouldNeverHappen) |
There was a problem hiding this comment.
Do we want panics or should we use errors? Same bellow
There was a problem hiding this comment.
the tree must be used with the prerequisites defined in the errShouldNeverHappen. If there is an error, no recovery should or can happen on these special cases indicating a significant bug in the logic. in such case, it seems better to panic.
| // TreeNode is a leaf compacted binary tree | ||
| // representing the address space of the neighborhood | ||
| type TreeNode[T any] struct { | ||
| K []byte |
There was a problem hiding this comment.
Are there cases were we would need concurrency suport via a mutex?
There was a problem hiding this comment.
there is no concurrent use of TreeNode at the moment so it does not require any concurrency handling for now
| func (t *peerTreeNode) BinAssignment() (peers []*peerTreeNodeValue) { | ||
| if t.isLeaf() { | ||
| bl := uint8(len(t.V.SyncBins)) | ||
| for i := t.L; i < bl; i++ { |
| syncPeer.syncBins = make([]bool, p.bins) | ||
| } | ||
| if po >= newRadius { | ||
| _ = bt.Put(addr.Bytes(), &peerTreeNodeValue{SyncBins: syncPeer.syncBins}) |
There was a problem hiding this comment.
Why we do not have also here changed = true ?
There was a problem hiding this comment.
because at this point the pullsync parameters may have not changed to reinit syncings but you want to build up the tree with the neighbor peers in case there was a change in the neighborhood peerset eventually.
bcsorvasi
added
bug
4 participants
This SWIP describes a more efficient way to synchronise content between peers in the same neighbourhood by syncing each chunk only once.
It removed the maxPOdelta exception for syncing bins from peers below storage depth. Accordingly, bee should not allow shallow receipts on pushing chunks. That should be tackled in a different PR.
Forwarding chunks to the closest address on the network is also required since chunks will be pulled from its closest full nodes.
This is the implementation of SWIP 25 for details check the proposal.
Checklist
Description
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):