-
Notifications
You must be signed in to change notification settings - Fork 110
Localstore integration #1282
Localstore integration #1282
Changes from 2 commits
df7d887
c0a4b1b
8381e38
5ac0f79
a96aeeb
44633ce
3e6e03a
6ce3481
5e69e3d
5d7f169
f0a8d3a
5474d30
9ca71f4
83b64ce
547f30b
970b305
5a669c6
4594130
4af1d7a
4df51ee
804301e
6aa0d0e
c16dcd1
14758a4
be1a360
36a0d96
3b3d2b0
4b8a5c8
277ecf9
682eebd
5ee9143
c3a76d1
2d20bf9
b828a34
d8c648b
e295dc8
c9d679d
2fe747e
3e82234
4c14a09
31f4b0d
2e24e99
9c79e26
ca7dcc2
e4d723c
904df7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -230,13 +230,21 @@ func (d *Delivery) handleChunkDeliveryMsg(ctx context.Context, sp *Peer, req int | |
| switch r := req.(type) { | ||
| case *ChunkDeliveryMsgRetrieval: | ||
| msg = (*ChunkDeliveryMsg)(r) | ||
| // do not sync if peer that is sending us a chunk is closer to the chunk then we are | ||
| peerPO := chunk.Proximity(sp.ID().Bytes(), msg.Addr) | ||
| po := chunk.Proximity(d.kad.BaseAddr(), msg.Addr) | ||
| if peerPO > po { | ||
| mode = chunk.ModePutRequest | ||
| } else { | ||
| depth := d.kad.NeighbourhoodDepth() | ||
| // is the chunk within our area of responsibility? | ||
| if po < depth { | ||
| // chunks within the area of responsibility should always sync | ||
| // https://github.com/ethersphere/go-ethereum/pull/1282#discussion_r269406125 | ||
| mode = chunk.ModePutSync | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand the rational here;
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a delivery request. If we got the chunk from the node that is closer to the chunk, we do not need to put in the storage to sync it. I am not sure what is a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sry i know i said this but it should only be the case for chunks outside our depth.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zelig thanks for the explanation, I may have understood wrongly. Could you check if current code handles various cases correctly?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zelig I was discussing this with @janos today and I have a few questions in this matter:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @justelad
|
||
| } else { | ||
| if peerPO > po { | ||
| // do not sync if peer that is sending us a chunk is closer to the chunk then we are | ||
| mode = chunk.ModePutRequest | ||
| } else { | ||
| mode = chunk.ModePutSync | ||
| } | ||
| } | ||
| case *ChunkDeliveryMsgSyncing: | ||
| msg = (*ChunkDeliveryMsg)(r) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is more compact representation. I thought that the current one was easier to reason about, but this is good with a comment, as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is good but you got the comparison wrong, note
po >= depth-> syncThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Viktor.