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): Properly shut down the store on control-c #2100

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Conversation

rklaehn
Copy link
Contributor

@rklaehn rklaehn commented Mar 20, 2024

Description

fix(iroh): Properly shut down the store on control-c

without this, the (flaky) cli_provide_persistence test fails, because
control-c interrupts the store before the write transaction is committed.

Notes & open questions

Should we change shutdown in node to be async, and remove the impl Future for Node?

Change checklist

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

@@ -1153,12 +1155,21 @@ impl StoreInner {
fn temp_file_name(&self) -> PathBuf {
self.path_options.temp_file_name()
}

async fn shutdown(&self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

God how I hate all this boilerplate. But no macros please. At some point I will write an actor framework to solve this...

without this, the (flaky) cli_provide_persistence test fails, because
control-c interrupts the store before the write transaction is committed.
## Description

Use the latest version of bao-tree that has some bugfixes and a new
validate fn.

## Notes & open questions

Due to the bugfix this means that it is no longer compatible with the
previous version of iroh-bytes in some rare circumstances involving
resume.

We would therefore have to increase the iroh-bytes APLN before the next
release.

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.
@rklaehn rklaehn requested a review from dignifiedquire March 20, 2024 14:21
# Conflicts:
#	iroh-bytes/src/store/bao_file.rs
@rklaehn rklaehn added this pull request to the merge queue Mar 20, 2024
Merged via the queue into main with commit 7cc9efa Mar 20, 2024
20 checks passed
@rklaehn rklaehn deleted the better-shutdown branch April 10, 2024 14:59
matheus23 pushed a commit that referenced this pull request Nov 14, 2024
## Description

fix(iroh): Properly shut down the store on control-c
    
without this, the (flaky) cli_provide_persistence test fails, because
control-c interrupts the store before the write transaction is
committed.

## Notes & open questions

Should we change shutdown in node to be async, and remove the impl
Future for Node?

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
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.

2 participants