mirror of
https://github.com/tursodatabase/limbo.git
synced 2025-12-23 08:21:09 +00:00
Merge 'Ignore SQLITE_BUSY during auto-checkpoint' from Mikaël Francoeur
## AI disclosure The draft of the description of this issue was written by Claude Code, and it also came up with the fix with just this instruction: >this fails, fix it > >cargo run -p limbo_sim -- --seed 12569318052915776741 --disable-bugbase --memory-io --minimum-tests 100 --maximum-tests 1000 loop -n 10 --short- circuit > >careful, the output is long. To see the error, you only need the last few dozen lines. I nudged it once, because it incorrectly said wrote that the only cause of a `SQLITE_BUSY` would be a concurrent checkpoint, and then I did some minor cleanup. I then started Claude Code in SQLite, to check if `SQLITE_BUSY` really was ignored during auto checkpoints, and it pointed me to [this line](https://github.com/sqlite/sqlite/blob/master/src/main.c#L2470), where the returned result code is indeed ignored. ## Description The issue was that when auto-checkpoint failed due to a Busy error, the entire commit would fail. However, at the point when checkpoint is called during commit, the WAL frames have already been written successfully, so the commit should proceed without error even if the checkpoint cannot run. This matches SQLite's behaviour, where auto- checkpointing uses the PASSIVE checkpoint mode (source), which ignores `SQLITE_BUSY` errors: > On the other hand, passive mode might leave the checkpoint unfinished if there are concurrent readers or writers. ([source](https://sqlite.org/c3ref/wal_checkpoint_v2.html)) The fix (in core/storage/pager.rs): When self.checkpoint() returns LimboError::Busy during the CommitState::Checkpoint phase of commit_dirty_pages_inner, we now: 1. Log a debug message that auto-checkpoint was skipped 2. Set the result to PagerCommitResult::WalWritten (indicating WAL was written but checkpoint didn't happen) 3. Continue to the Done state to complete the commit successfully Closes https://github.com/tursodatabase/turso/issues/4059 Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #4118
This commit is contained in:
commit
5c8176da25
1 changed files with 17 additions and 3 deletions
|
|
@ -1939,8 +1939,22 @@ impl Pager {
|
|||
self.commit_info.write().state = CommitState::Checkpoint;
|
||||
}
|
||||
CommitState::Checkpoint => {
|
||||
match self.checkpoint()? {
|
||||
IOResult::IO(cmp) => {
|
||||
match self.checkpoint() {
|
||||
Err(LimboError::Busy) => {
|
||||
// Auto-checkpoint during commit uses Passive mode, which can return
|
||||
// Busy if either:
|
||||
// 1. Another connection is already checkpointing (checkpoint_lock held)
|
||||
// 2. A reader is active on read slot 0 (read_locks[0] held)
|
||||
// In either case, skip the checkpoint and complete the commit. The WAL
|
||||
// frames are already written, so the commit succeeds. Checkpoint will
|
||||
// happen later when conditions allow.
|
||||
tracing::debug!("Auto-checkpoint skipped due to busy (lock conflict)");
|
||||
let mut commit_info = self.commit_info.write();
|
||||
commit_info.result = Some(PagerCommitResult::WalWritten);
|
||||
commit_info.state = CommitState::Done;
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
Ok(IOResult::IO(cmp)) => {
|
||||
let completion = {
|
||||
let mut commit_info = self.commit_info.write();
|
||||
match cmp {
|
||||
|
|
@ -1957,7 +1971,7 @@ impl Pager {
|
|||
// TODO: remove serialization of checkpoint path
|
||||
io_yield_one!(completion);
|
||||
}
|
||||
IOResult::Done(res) => {
|
||||
Ok(IOResult::Done(res)) => {
|
||||
let mut commit_info = self.commit_info.write();
|
||||
commit_info.result = Some(PagerCommitResult::Checkpointed(res));
|
||||
// Skip sync if synchronous mode is OFF
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue