-
Notifications
You must be signed in to change notification settings - Fork 254
fix(provider): conflict resolution #1199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| github.com/ipfs/go-cid v0.5.0 | ||
| github.com/ipfs/go-datastore v0.9.0 | ||
| github.com/ipfs/go-detect-race v0.0.1 | ||
| github.com/ipfs/go-ds-pebble v0.5.6 |
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.
nit: this pulls in extra deps, and we already had leveldb in dependency tree, but i think we need to use pebble because leveldb is not compatible with testing/synctest (?)
either way, merging, we can cleanup before tagging final release (waiting with that until after kubo 0.39.0-rc1 (we will do kad-dht release only before final kubo 0.39)
I will switch to latest kubo master and test in ipfs/kubo#10955
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.
We can try using leveldb and if it works fine, no need to pull pebble. The goal of the test is to persist state on disk (not only MapDatastore), and restore that state from disk.
| // This is a corner case that is required for Kubo, as Keystore.Reset() | ||
| // takes a while. | ||
| s.reprovideQueue.Enqueue(keyspace.AllKeys(s.schedule, s.order)...) | ||
| s.catchupPendingWork() |
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.
(note to self why we need it)
Without catchupPendingWork():
- Kubo calls Keystore.Reset() with new CIDs (could be a slow operation)
- RefreshSchedule() enqueues ALL regions to reprovideQueue
- Work sits idle in the queue waiting for the next periodic retry (could be minutes away)
- New content is not announced to the DHT for an extended period
- The content is effectively invisible to the network during this window
With catchupPendingWork():
- Kubo calls Keystore.Reset() with new CIDs
- RefreshSchedule() enqueues ALL regions to reprovideQueue
- Immediately spawns goroutine to process the queue
- New content is announced to DHT as soon as possible
- Content becomes discoverable immediately
Recent conflict resolution (probably in #1193) removed some code.
This PR restores the removed code. Hopefully it doesn't miss anything.
I had already resolved some conflicts between the recent PRs in branch
recent-changes-tmpused in https://github.com/ipshipyard/waterworks-infra/pull/821, but it didn't include #1194 at the time.I used the command
git diff a53fc23f77f4e2ec87ce733b8efaabce38ecf8f9 1acbd9bto identify removed code, and filtering out connectivity related changes (introduced in #1194).