-
Notifications
You must be signed in to change notification settings - Fork 213
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
feat(iroh-sync): read only replicas #1770
Conversation
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.
Looks good! Couple of minor nits in the review comments.
One question though:
If reading the current code correctly, importing a read capability (e.g. from a ticket) for an existing, writable namespace would delete a namespace's secret key. This is not the desired outcome, I'd say. What should happen IMO:
- Import write capability for existing namespace with read capability: Upgrade to write capability
- Import read capability for existing namespace with write capability: No change (but also no error)
Would be great to change this and add a test.
I added code to merge capabilities. When importing a write capability for a namespace for which you have a read-only capability currently, it will now merge the capabilities in the store. however, if an actual Replica instance is opened in the sync actor, it will not yet update the capability to write, which is a problem. How should we solve this? Currently, the store does not keep a copy of the replica (it is non-cloneable, this was changed in the actor pr). What we could do is that the actor would both do store.import_namespace and replica.update_capability or so. wdyt? |
I pushed another commit that takes care of upgrading capabilities of open replicas. |
## Description The migration as commited with #1770 panics when starting iroh with an existing data dir. The reason is that redb panics when deleting table in a transaction where they were opened as well. See: cberner/redb#715 This fixes this by moving the table deletion to a separate transaction. The limitation and potential panic in redb was fixed in cberner/redb#716 so once we upgrade to the next (still unreleased) version of redb, this can be removed again. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant.
## Description Closes #1767 Updates sync, the sync db, commands and tickets to enable read only replicas ## Notes & open questions <!-- Any notes, remarks or open questions you have to make about the PR. --> ## Change checklist - [ ] Self-review. - [ ] Documentation updates if relevant. - [ ] Tests if relevant. --------- Co-authored-by: Franz Heinzmann (Frando) <[email protected]>
Description
Closes #1767
Updates sync, the sync db, commands and tickets to enable read only replicas
Notes & open questions
Change checklist