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

pushsync, storage/localstore: set tags synced state in localstore #1821

Closed
wants to merge 1 commit into from

Conversation

janos
Copy link
Member

@janos janos commented Sep 26, 2019

Do not merge.

This is one example solution for tags to get along with both push and pull syncing. Tag synced state cannot reach total number of chunks as pull syncing is removing chunks from push index on ModeSetSync.

It is debatable if it is correct approach as push and pull syncing have different definition of chunk being synced. But it is the most simple thing to do now.

@zelig
Copy link
Member

zelig commented Sep 26, 2019

@janos this solution is looks elegant, and makes it an independent decision whether pullsync should ever call setsynced.
but pullsyc should increment Sent on the tag and even that only i no pushsync is enabled.
If pushsync is enabled, pullsync should never call set cos that renders the synced state meaningless
soon we must make pushsync option a runtime option, this solution can help implementing it simply via tags. if the tag says pullsync only, increment sent count.
(note that two modesets dont help as the sync provider has no access to info about the pushsync intent of the uploader)
to be honest since both types of sync index persist across sessions, i have a hard time interpreting the sync modes as cli params probably should replace it with runtime upload option recorded on the ta

@acud
Copy link
Member

acud commented Sep 27, 2019

two major points from my side:

  1. is pushsync opt-in for each node that runs? i think we should conclude if this is opt-in at all, as this will create problems in delivery
  2. as an uploader, if i want to track my upload and know when it is retrievable - i assume only pushysnc should be used for the purpose of syncing the content, as pullsync does not build on neither of the aforementioned premises

the PR solves the problem of the counters being incremented both by push and pull running at the same time, but it hinders the ability to know when a chunk is retrievable correctly from NN (i.e. the chunks would all be marked as synced but not all are synced by pushsync hence not all are retrievable). please correct me if i'm wrong

case nil:
tag, _ := db.tags.Get(i.Tag)
if tag != nil {
tag.Inc(chunk.StateSynced)
Copy link
Member

Choose a reason for hiding this comment

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

i think you can also Inc(chunk.StateSent)

@janos
Copy link
Member Author

janos commented Oct 16, 2019

Closing as it was obsoleted by #1828.

@janos janos closed this Oct 16, 2019
@janos janos deleted the localstore-tags-sync branch October 16, 2019 07:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants