infiniflow / infiniflow/infinity

[Bug]: WAL flush thread may drop queued entries during shutdown

Open
#3,375 1 comment 0 reactions 1 assignee Claimed by @qinling0210 View on GitHub
bug
Dominant language
C++
Stars
4.7k
Forks
445
Avg merge
2d 2h
Merged PRs (30d)
7

Description

### Is there an existing issue for the same bug?

- [x] I have checked the existing issues.

### Version or Commit ID

main branch

### Other environment information

```Markdown

```

### Actual behavior and How to reproduce it

## Actual behavior

`WalManager::NewFlush()` may exit before draining queued WAL entries during shutdown.

The current implementation uses both:

* global `running_` flag
* `nullptr` queue sentinel

Under certain timing conditions, queued WAL entries can be skipped during shutdown.

Current shutdown flow:

```cpp
Stop():
running_ = false;
Enqueue(nullptr);

Flush thread:
DequeueBulk(batch);

...

if (!running_.load()) {
break;
}
```

This allows the flush thread to exit before processing all remaining queued entries.

Additionally, when `nullptr` appears inside `txn_batch`, the current loop immediately breaks:

```cpp
if (txn == nullptr) {
running_ = false;
break;
}
```

This may skip entries behind the sentinel inside the same batch.

## How to reproduce

The issue is timing-dependent but can happen during concurrent WAL activity + shutdown.

One possible reproduction flow:

1. Start server
2. Continuously submit transactions
3. Trigger shutdown while WAL flush thread is processing batches
4. Restart and replay WAL
5. Observe missing committed entries

### Expected behavior

During shutdown, the WAL flush thread should fully drain all queued WAL entries before exiting.

All non-nullptr queued entries should always be:

* serialized
* written to WAL
* flushed

before the flush thread terminates.

### Additional information

I have a local fix using a queue-draining shutdown model:

* `running_` only controls whether new transactions are accepted
* `nullptr` is treated as a shutdown marker
* flush thread exits only after the current batch is fully drained

I also validated the fix with shutdown/recovery tests locally.

Will open a PR soon.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.