Skip to content

Commit

Permalink
fix(iroh-docs)!: Add flush_store and use it to make sure the defaul…
Browse files Browse the repository at this point in the history
…t author is persisted (#2471)

## Description

<!-- A summary of what this pull request achieves and a rough list of
changes. -->
- 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:
```rust
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

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->
None. Only an addition: `iroh-docs::actor::SyncHandle::flush_store`.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

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

Closes #2462 

## Change checklist

- [X] Self-review.
- [X] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- ~~[ ] Tests if relevant.~~
- [X] All breaking changes documented.
  • Loading branch information
matheus23 authored Jul 8, 2024
1 parent 235c69c commit b88dfa5
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 0 deletions.
21 changes: 21 additions & 0 deletions iroh-docs/src/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ enum Action {
#[debug("reply")]
reply: oneshot::Sender<Result<ContentHashesIterator>>,
},
#[display("FlushStore")]
FlushStore {
#[debug("reply")]
reply: oneshot::Sender<Result<()>>,
},
#[display("Replica({}, {})", _0.fmt_short(), _1)]
Replica(NamespaceId, ReplicaAction),
#[display("Shutdown")]
Expand Down Expand Up @@ -542,6 +547,21 @@ impl SyncHandle {
rx.await?
}

/// Makes sure that all pending database operations are persisted to disk.
///
/// Otherwise, database operations are batched into bigger transactions for speed.
/// Use this if you need to make sure something is written to the database
/// before another operation, e.g. to make sure sudden process exits don't corrupt
/// your application state.
///
/// It's not necessary to call this function before shutdown, as `shutdown` will
/// trigger a flush on its own.
pub async fn flush_store(&self) -> Result<()> {
let (reply, rx) = oneshot::channel();
self.send(Action::FlushStore { reply }).await?;
rx.await?
}

async fn send(&self, action: Action) -> Result<()> {
self.tx
.send_async(action)
Expand Down Expand Up @@ -675,6 +695,7 @@ impl Actor {
Action::ContentHashes { reply } => {
send_reply_with(reply, self, |this| this.store.content_hashes())
}
Action::FlushStore { reply } => send_reply(reply, self.store.flush()),
Action::Replica(namespace, action) => self.on_replica_action(namespace, action),
}
}
Expand Down
4 changes: 4 additions & 0 deletions iroh-docs/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ impl DefaultAuthorStorage {
let author = Author::new(&mut rand::thread_rng());
let author_id = author.id();
docs_store.import_author(author).await?;
// Make sure to write the default author to the store
// *before* we write the default author ID file.
// Otherwise the default author ID file is effectively a dangling reference.
docs_store.flush_store().await?;
self.persist(author_id).await?;
Ok(author_id)
}
Expand Down

0 comments on commit b88dfa5

Please sign in to comment.