Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

chunk: fix tag Sent value on load, dont persist done tags #1914

Merged
merged 1 commit into from
Nov 3, 2019

Conversation

acud
Copy link
Member

@acud acud commented Oct 29, 2019

This PR fixes a state where a chunk was sent with push sync but for which a receipt was not received due to node shutdown. Thus, it is mandatory to set the tag.Sent counter to the tag.Synced counter, so that the next send of the same chunk (which still exists in push sync index) is accounted for correctly.

Another change introduced here is that Done tags should not be persisted. I am open to discuss this but it seemed like the right thing to do at the lack of some sort of pruning mechanism. I am considering either this approach or an approach where only tags which are done and older than X days should not be persisted across sessions. Inputs would be appreciated.

@acud acud added the bug label Oct 29, 2019
@acud acud added this to the 0.5.3 milestone Oct 29, 2019
@acud acud requested review from janos and zelig October 29, 2019 11:13
@acud acud self-assigned this Oct 29, 2019
@acud acud added this to Backlog in Swarm Core - Sprint planning via automation Oct 29, 2019
@acud acud moved this from Backlog to In review (includes Documentation) in Swarm Core - Sprint planning Oct 29, 2019

// prevent a condition where a chunk was sent before shutdown
// and the node was turned off before the receipt was received
v.Sent = v.Synced
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there would be similar data inconsistencies that we could not notice, which would result in strange behaviour. This is a sign that persistence is not reliable.

Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are fine, but I am worried if persistence is reliable enough in the current form.

@acud acud merged commit 21ac3ec into master Nov 3, 2019
Swarm Core - Sprint planning automation moved this from In review (includes Documentation) to Done Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants