Skip to content
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

Merged
merged 18 commits into from
Nov 6, 2023

Conversation

divagant-martian
Copy link
Contributor

Description

Closes #1767
Updates sync, the sync db, commands and tickets to enable read only replicas

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

@dignifiedquire dignifiedquire added this to the v0.10.0 milestone Nov 6, 2023
Copy link
Member

@Frando Frando left a 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.

iroh-sync/src/actor.rs Outdated Show resolved Hide resolved
iroh-sync/src/store/fs.rs Outdated Show resolved Hide resolved
iroh-sync/src/store/memory.rs Outdated Show resolved Hide resolved
iroh-sync/src/store/fs.rs Outdated Show resolved Hide resolved
@Frando
Copy link
Member

Frando commented Nov 6, 2023

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?

@Frando
Copy link
Member

Frando commented Nov 6, 2023

I pushed another commit that takes care of upgrading capabilities of open replicas.

@Frando Frando enabled auto-merge November 6, 2023 18:37
@Frando Frando added this pull request to the merge queue Nov 6, 2023
Merged via the queue into n0-computer:main with commit c1ebea8 Nov 6, 2023
15 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Nov 7, 2023
## 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.
@divagant-martian divagant-martian deleted the read-only-replica branch December 10, 2023 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Read-only replica sharing
3 participants