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

fix(iroh-docs): Add flush_store and use it to make sure the default author is persisted #2471

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

matheus23
Copy link
Member

Description

  • Add a flush_store function to the docs actor/handle
  • Use docs_store.flush_store().await? before persisting the default author ID file

There's two pieces of data related to the default author:

  • The default author ID file
  • The actual default author (the secret key) in redb

The code to store these two pieces of data looked like this:

docs_store.import_author(author).await?;
self.persist(author_id).await?;

So it would look like we make sure the author is stored before we store the file (which is basically just a reference to the author in the DB).

However, import_author is somewhat lazy, because it batches operations together in a bigger transaction.
The store would actually only get flushed in e.g. shutdown. So most of the time, the actual persisting of the author would happen after the call to self.persist.

If you wait for shutdown every time, it's all good, because that'll call flush. If not, then there will be some issues.


The fix in this PR is to introduce a flush_store function so we can use it after import_author to make sure it's actually flushed before we self.persist(author_id).

Breaking Changes

None. Only an addition: iroh-docs::actor::SyncHandle::flush_store.

Notes & open questions

There might still be something wrong with shutdown not running correctly, but IMO this is an improvement regardless.

Closes #2462

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • [ ] Tests if relevant.
  • All breaking changes documented.

@matheus23 matheus23 self-assigned this Jul 8, 2024
@matheus23
Copy link
Member Author

Before this PR, the cli_rpc_lock_restart test would fail "reliably".
After this PR, it runs fine locally.
It may still be flaky. I don't know. But at least it's not "broken for sure".

@matheus23 matheus23 added this pull request to the merge queue Jul 8, 2024
@dignifiedquire dignifiedquire changed the title fix(iroh-docs)!: Add flush_store and use it to make sure the default author is persisted fix(iroh-docs): Add flush_store and use it to make sure the default author is persisted Jul 8, 2024
Merged via the queue into main with commit b88dfa5 Jul 8, 2024
27 checks passed
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.

bug: "The default author is missing from the docs store, to recover delete the file [...]"
2 participants