feat: pusher skipping syncing for chunks with expired stamps #2392
Conversation
istae
changed the title
istae
changed the title
istae
changed the title
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @acud, @aloknerurkar, and @istae)
pkg/pusher/pusher_test.go, line 36 at r1 (raw file):
var noOfRetries = 20 var block = common.HexToHash("0x1").Bytes() var defaultValidStamp = func(ch swarm.Chunk, stamp []byte) (swarm.Chunk, error) {
call it mock no?
pkg/pusher/pusher_test.go, line 465 at r1 (raw file):
} // Check is the chunk is set as synced in the DB.
check IF typo
| _, err = s.validStamp(ch, stampBytes) | ||
| if err != nil { | ||
| s.logger.Warningf("pusher: stamp with batch ID %x is no longer valid, skipping syncing for chunk %s: %v", ch.Stamp().BatchID(), ch.Address().String(), err) | ||
| if err = s.storer.Set(ctx, storage.ModeSetSync, ch.Address()); err != nil { |
There was a problem hiding this comment.
I know this is what was agreed on, however, I do not like this approach of marking the chunk as synced. For a user of bee, this will result in incorrect information on the /tags API. He would assume the chunk is synced. The only way to know for sure right now is to keep monitoring logs for this warning msg.
The problem is while designing dapps, they might rely on the /tags endpoint to possibly show some status to their users and this will break their applications. Maybe we should consider adding a new state here which can be returned on the tags endpoint, this will allow developers to take some corrective action.
There was a problem hiding this comment.
@aloknerurkar it is a good point but I'm not sure how a new state would resolve things. I personally don't think that our storage abstraction can deal with this correctly right now even with the addition of a new mode, since we would need more ways to communicate the information to the users.
TBH from my side the best solution would be to revive the concept of uploading directly to the network, this would allow easier error propagation to the user with regards to network errors or postage stamp validity.
There was a problem hiding this comment.
For now, I think we should at least add some comments here to mention this hack and come back to it later.
This change is