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

Avoid recovery when using Durability::Paranoid #878

Open
mconst opened this issue Oct 18, 2024 · 4 comments
Open

Avoid recovery when using Durability::Paranoid #878

mconst opened this issue Oct 18, 2024 · 4 comments

Comments

@mconst
Copy link

mconst commented Oct 18, 2024

Currently, redb sets the needs_recovery flag as soon as you open the database, which means any unclean shutdown will trigger recovery. For large databases, this isn't a great experience -- it means a simple Ctrl-C can end up locking you out of your database for a half-hour or more while it recovers. It would be really nice if there were a way to avoid this.

Of course, recovery is unavoidable after an unclean shutdown when using Durability::Immediate, since there's no way to know whether the last commit was complete. But if you're using Durability::Paranoid, it seems like recovery shouldn't be necessary! This would let applications opt into slower commits in exchange for not needing recovery, which would be a big usability win when the database is large.

To make this work, I believe redb would need to write out the allocator state as part of Durability::Paranoid commits (or this could be a new durability level above Paranoid, to avoid slowing down existing Durability::Paranoid users who don't care about recovery speed). But I think it could be done in a fully backwards- and forwards-compatible way.

Does this sound right to you? And if so, do you think it would be a worthwhile change?

@cberner
Copy link
Owner

cberner commented Oct 19, 2024

Ya, that seems useful. It should probably be separate from Paranoid since the allocator state grows linearly in the size of the database, and could be quite large.

Do you think a new Durability level, or a separate Database::flush(&self) method is more idiomatic?

@mconst
Copy link
Author

mconst commented Oct 19, 2024

Either way works, but I think a new Durability level would be nicer. This minimizes the performance impact (since calling Database::flush() would mean an extra fsync(), whereas a new Durability level lets us do it as part of the regular two-phase commit), and it also avoids having a gap in between commit() and Database::flush() where a crash would still require recovery.

If it's useful, I can describe the design I had in mind -- I haven't implemented it yet, but I think I have a decent idea of how it would work.

@cberner
Copy link
Owner

cberner commented Oct 19, 2024

Ya, a new Durability level seems good to me. Happy to merge a PR if you want to implement it!

@mconst
Copy link
Author

mconst commented Oct 21, 2024

Awesome! I'll try implementing this and I'll send you a PR in a week or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants