fix(mem_wal): prevent cross-generation durability acknowledgements#7759
fix(mem_wal): prevent cross-generation durability acknowledgements#7759u70b3 wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDurable MemTable writes now use request-scoped WAL flush completion cells, preventing acknowledgements from older generations. Watchers propagate successful completion, typed failures, and handler-exit errors, with unit and integration coverage. ChangesWAL durability watcher flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant put_memtable_no_wait
participant WalFlusher
participant WatchableOnceCell
participant BatchDurableWatcher
Client->>put_memtable_no_wait: durable put
put_memtable_no_wait->>WatchableOnceCell: create request completion
put_memtable_no_wait->>WalFlusher: trigger_flush with captured batch range
WalFlusher->>WatchableOnceCell: write flush success or typed failure
BatchDurableWatcher->>WatchableOnceCell: await request completion
WatchableOnceCell-->>BatchDurableWatcher: return durability result
BatchDurableWatcher-->>Client: resolve durability or failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
6e30389 to
43a408f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance/src/dataset/mem_wal/wal.rs (1)
91-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDesign is sound; doc comments lack examples and enum-variant semantics.
The request-scoped
FlushCompletionbackend correctly resolves the cross-generation aliasing bug (completion is tied to the specific triggeringBatchStore/request rather than a shared watermark), andwait()/is_durable()correctly branch per backend. Two doc gaps per the coding guideline:
BatchDurabilityState::Watermark/FlushCompletion(Lines 92-101) have no doc comments explaining what each variant represents.- The public
BatchDurableWatcherstruct/wait()/is_durable()docs (Lines 103-110, 137-144, 179-180) describe the concept but include no runnable example and don't link to sibling methods (e.g.[Self::wait]/[Self::is_durable]).📝 Example doc additions
enum BatchDurabilityState { + /// Legacy watermark-based tracking: durable once the shared watermark + /// reaches `target_batch_position`. Watermark { rx: watch::Receiver<usize>, target_batch_position: usize, terminal_error: Arc<StdMutex<Option<WalFlushFailure>>>, }, + /// Request-scoped tracking: durable only when the specific triggering + /// flush (tied to one `BatchStore` generation) completes. FlushCompletion { reader: WatchableOnceCellReader<std::result::Result<WalFlushResult, WalFlushFailure>>, }, }As per coding guidelines, "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods" and "Document enum variants with behavioral semantics, not just labels."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/mem_wal/wal.rs` around lines 91 - 221, Add documentation for both BatchDurabilityState variants describing their behavioral semantics, including watermark tracking versus request-scoped flush completion. Expand BatchDurableWatcher, wait, and is_durable documentation with runnable Rust examples and cross-links to related methods such as [Self::wait] and [Self::is_durable], while preserving the existing API behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@rust/lance/src/dataset/mem_wal/wal.rs`:
- Around line 91-221: Add documentation for both BatchDurabilityState variants
describing their behavioral semantics, including watermark tracking versus
request-scoped flush completion. Expand BatchDurableWatcher, wait, and
is_durable documentation with runnable Rust examples and cross-links to related
methods such as [Self::wait] and [Self::is_durable], while preserving the
existing API behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a880222-4c62-4ea8-b5c8-60418127b0a6
📒 Files selected for processing (4)
rust/lance/src/dataset/mem_wal/wal.rsrust/lance/src/dataset/mem_wal/write.rsrust/lance/tests/integration_tests.rsrust/lance/tests/mem_wal.rs
43a408f to
9ae0702
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance/src/dataset/mem_wal/wal.rs`:
- Around line 103-107: Expand the public watcher documentation around the
watcher type and its methods to include a runnable usage example showing
construction, awaiting durability, and checking status. Add intra-doc links to
Self::new, Self::wait, Self::is_durable, and the relevant ShardWriter method,
preserving the existing compatibility and request-scoped durability behavior.
In `@rust/lance/tests/mem_wal.rs`:
- Around line 12-14: Standardize all affected memory WAL tests on the canonical
in-memory URI: in rust/lance/tests/mem_wal.rs lines 12-14, replace the temporary
directory and filesystem URI setup with ObjectStore::from_uri("memory://"); in
rust/lance/src/dataset/mem_wal/write.rs line 3705, replace create_local_store()
with an in-memory object store; and at line 4890, change memory:/// to
memory://. Do not add counters or unique URI suffixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2f78ee21-5656-40a8-bccf-0b9b9f22ac32
📒 Files selected for processing (4)
rust/lance/src/dataset/mem_wal/wal.rsrust/lance/src/dataset/mem_wal/write.rsrust/lance/tests/integration_tests.rsrust/lance/tests/mem_wal.rs
| /// Watcher for batch durability. | ||
| /// | ||
| /// Uses a shared watch channel that broadcasts the durable watermark. | ||
| /// The watcher waits until the watermark reaches or exceeds its target batch ID. | ||
| /// Publicly constructed watchers retain watermark-based tracking for | ||
| /// compatibility. MemTable writes use a request-scoped flush completion so a | ||
| /// watermark from another generation cannot acknowledge their data. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add an example and intra-doc links for the public watcher API.
Document how callers obtain and await this watcher, and link [Self::new], [Self::wait], [Self::is_durable], and the relevant ShardWriter method.
As per coding guidelines: “All public APIs must have documentation with examples, and documentation should link to relevant structs and methods.”
Also applies to: 114-127, 137-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/src/dataset/mem_wal/wal.rs` around lines 103 - 107, Expand the
public watcher documentation around the watcher type and its methods to include
a runnable usage example showing construction, awaiting durability, and checking
status. Add intra-doc links to Self::new, Self::wait, Self::is_durable, and the
relevant ShardWriter method, preserving the existing compatibility and
request-scoped durability behavior.
Source: Coding guidelines
| let temp_dir = tempfile::tempdir().unwrap(); | ||
| let base_uri = format!("file://{}", temp_dir.path().display()); | ||
| let (store, base_path) = ObjectStore::from_uri(&base_uri).await.unwrap(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Standardize the new tests on the canonical memory:// URI.
rust/lance/tests/mem_wal.rs#L12-L14: replace the temporary filesystem store withObjectStore::from_uri("memory://").rust/lance/src/dataset/mem_wal/write.rs#L3705-L3705: replacecreate_local_store()with an in-memory object store.rust/lance/src/dataset/mem_wal/write.rs#L4890-L4890: changememory:///tomemory://.
As per coding guidelines: “Use plain "memory://" URIs in tests; no atomic counters or unique suffixes are needed.”
📍 Affects 2 files
rust/lance/tests/mem_wal.rs#L12-L14(this comment)rust/lance/src/dataset/mem_wal/write.rs#L3705-L3705rust/lance/src/dataset/mem_wal/write.rs#L4890-L4890
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/tests/mem_wal.rs` around lines 12 - 14, Standardize all affected
memory WAL tests on the canonical in-memory URI: in rust/lance/tests/mem_wal.rs
lines 12-14, replace the temporary directory and filesystem URI setup with
ObjectStore::from_uri("memory://"); in rust/lance/src/dataset/mem_wal/write.rs
line 3705, replace create_local_store() with an in-memory object store; and at
line 4890, change memory:/// to memory://. Do not add counters or unique URI
suffixes.
Source: Coding guidelines
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance/src/dataset/mem_wal/wal.rs (1)
144-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWatcher docs still lack an example and full cross-links.
The new
[Self::is_durable]/[Self::wait]cross-links are a good partial fix, but the publicBatchDurableWatcherAPI (new,wait,is_durable) still has no runnable usage example, and neither method links back toSelf::newor the relevantShardWritermethod that constructs/consumes it. This is the same gap flagged in the prior review round.As per coding guidelines: "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/mem_wal/wal.rs` around lines 144 - 203, The public BatchDurableWatcher API documentation needs a runnable usage example and complete cross-links. Update the docs for BatchDurableWatcher::new, BatchDurableWatcher::wait, and BatchDurableWatcher::is_durable to include an appropriate example and links to Self::new plus the relevant ShardWriter method that creates or consumes the watcher; retain the existing wait/is_durable relationship links.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@rust/lance/src/dataset/mem_wal/wal.rs`:
- Around line 144-203: The public BatchDurableWatcher API documentation needs a
runnable usage example and complete cross-links. Update the docs for
BatchDurableWatcher::new, BatchDurableWatcher::wait, and
BatchDurableWatcher::is_durable to include an appropriate example and links to
Self::new plus the relevant ShardWriter method that creates or consumes the
watcher; retain the existing wait/is_durable relationship links.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0f69c551-d4b2-407c-9033-c5f42bdeaabb
📒 Files selected for processing (2)
rust/lance/src/dataset/mem_wal/wal.rsrust/lance/src/dataset/mem_wal/write.rs
eb53323 to
ba772ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
rust/lance/tests/mem_wal.rs (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
memory://instead of a temp-dirfile://store.Per repo convention this new integration test should use an in-memory object store rather than a temporary filesystem directory.
As per coding guidelines: "Use plain
\"memory://\"URIs in tests; no atomic counters or unique suffixes are needed."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/tests/mem_wal.rs` around lines 12 - 14, Update the test setup around ObjectStore::from_uri to use the plain memory:// URI instead of creating a tempfile directory and constructing a file:// URI. Remove the unnecessary temp_dir and base_uri setup while preserving the existing store and base_path initialization.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance/src/dataset/mem_wal/wal.rs`:
- Around line 144-203: Add runnable Rust doc examples to the public
BatchDurableWatcher methods wait and is_durable, showing a caller with an
obtained BatchDurableWatcher checking is_durable and awaiting wait when needed.
Keep the example compilable using the existing lance::Result and watcher types,
and retain links to the related methods.
In `@rust/lance/src/dataset/mem_wal/write.rs`:
- Around line 4897-4900: Update the base_uri value in
test_durable_watcher_does_not_alias_across_memtable_generations from the
noncanonical "memory:///" form to the plain "memory://" form, preserving the
rest of the test unchanged.
- Around line 3706-3715: The new test
test_coalesced_empty_flush_completes_request_watcher should use an in-memory
store with the plain memory:// URI instead of create_local_store() and its
temporary file-backed setup. Remove the temp-directory dependency while
preserving the existing WAL appender/flusher test behavior.
---
Duplicate comments:
In `@rust/lance/tests/mem_wal.rs`:
- Around line 12-14: Update the test setup around ObjectStore::from_uri to use
the plain memory:// URI instead of creating a tempfile directory and
constructing a file:// URI. Remove the unnecessary temp_dir and base_uri setup
while preserving the existing store and base_path initialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e455bee4-9e08-450b-a161-8237397f096f
📒 Files selected for processing (4)
rust/lance/src/dataset/mem_wal/wal.rsrust/lance/src/dataset/mem_wal/write.rsrust/lance/tests/integration_tests.rsrust/lance/tests/mem_wal.rs
| /// Wait until the watched batch is durable. | ||
| /// | ||
| /// Watermark-backed watchers return success when the durable watermark | ||
| /// reaches the target, and return an error for a terminal flush failure or | ||
| /// closed tracking channel. Request-scoped watchers return success only | ||
| /// after their flush completes, preserve typed flush failures, and return an | ||
| /// I/O error if the flush handler exits without reporting completion. | ||
| /// | ||
| /// Returns Ok(()) when `durable_watermark >= target_batch_position`, or | ||
| /// Err if a terminal flush failure (e.g. a fence) means the watermark can | ||
| /// never reach the target. | ||
| /// For a non-blocking check see [`Self::is_durable`]. | ||
| pub async fn wait(&mut self) -> Result<()> { | ||
| loop { | ||
| if let Some(failure) = self.terminal_error.lock().unwrap().clone() { | ||
| return Err(failure.into_error()); | ||
| } | ||
| let current = *self.rx.borrow(); | ||
| if current >= self.target_batch_position { | ||
| return Ok(()); | ||
| } | ||
| self.rx | ||
| .changed() | ||
| .await | ||
| .map_err(|_| Error::io("Durable watermark channel closed"))?; | ||
| match &mut self.state { | ||
| BatchDurabilityState::Watermark { | ||
| rx, | ||
| target_batch_position, | ||
| terminal_error, | ||
| } => loop { | ||
| let terminal_failure = terminal_error | ||
| .lock() | ||
| .map_err(|_| { | ||
| Error::internal( | ||
| "Batch durability terminal error mutex was poisoned".to_string(), | ||
| ) | ||
| })? | ||
| .clone(); | ||
| if let Some(failure) = terminal_failure { | ||
| return Err(failure.into_error()); | ||
| } | ||
| if *rx.borrow() >= *target_batch_position { | ||
| return Ok(()); | ||
| } | ||
| rx.changed() | ||
| .await | ||
| .map_err(|_| Error::io("Durable watermark channel closed"))?; | ||
| }, | ||
| BatchDurabilityState::FlushCompletion { reader } => match reader.await_value().await { | ||
| Some(Ok(_)) => Ok(()), | ||
| Some(Err(failure)) => Err(failure.into_error()), | ||
| None => Err(Error::io( | ||
| "WAL flush handler exited before reporting durability", | ||
| )), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| /// Check if the batch is already durable (non-blocking). | ||
| /// | ||
| /// Cheaper, non-awaiting counterpart to [`Self::wait`]: returns the | ||
| /// current state without blocking for the flush to complete. | ||
| pub fn is_durable(&self) -> bool { | ||
| *self.rx.borrow() >= self.target_batch_position | ||
| match &self.state { | ||
| BatchDurabilityState::Watermark { | ||
| rx, | ||
| target_batch_position, | ||
| .. | ||
| } => *rx.borrow() >= *target_batch_position, | ||
| BatchDurabilityState::FlushCompletion { reader } => { | ||
| matches!(reader.read(), Some(Ok(_))) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Doc examples still missing for wait()/is_durable().
Intra-doc links to [Self::wait]/[Self::is_durable] were added since the last review, but neither method has a runnable doc example showing how a caller constructs/obtains a watcher, awaits durability, and checks status.
As per coding guidelines: "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods."
📚 Suggested example
/// # Example
/// ```
/// # use lance::dataset::mem_wal::BatchDurableWatcher;
/// # async fn doc(mut watcher: BatchDurableWatcher) -> lance::Result<()> {
/// if !watcher.is_durable() {
/// watcher.wait().await?;
/// }
/// # Ok(())
/// # }
/// ```
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/src/dataset/mem_wal/wal.rs` around lines 144 - 203, Add runnable
Rust doc examples to the public BatchDurableWatcher methods wait and is_durable,
showing a caller with an obtained BatchDurableWatcher checking is_durable and
awaiting wait when needed. Keep the example compilable using the existing
lance::Result and watcher types, and retain links to the related methods.
Source: Coding guidelines
| #[tokio::test] | ||
| async fn test_coalesced_empty_flush_completes_request_watcher() { | ||
| let (store, base_path, _base_uri, _temp_dir) = create_local_store().await; | ||
| let shard_id = Uuid::new_v4(); | ||
| let wal_appender = Arc::new( | ||
| WalAppender::open(store, base_path, shard_id, 0) | ||
| .await | ||
| .unwrap(), | ||
| ); | ||
| let wal_flusher = Arc::new(WalFlusher::new(wal_appender)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use memory:// instead of a temp-dir file:// store for this new test.
create_local_store() backs this test with a real filesystem temp dir; per repo convention new tests should use an in-memory store instead.
As per coding guidelines: "Use plain \"memory://\" URIs in tests; no atomic counters or unique suffixes are needed."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/src/dataset/mem_wal/write.rs` around lines 3706 - 3715, The new
test test_coalesced_empty_flush_completes_request_watcher should use an
in-memory store with the plain memory:// URI instead of create_local_store() and
its temporary file-backed setup. Remove the temp-directory dependency while
preserving the existing WAL appender/flusher test behavior.
Source: Coding guidelines
| async fn test_durable_watcher_does_not_alias_across_memtable_generations() { | ||
| let (store, base_path, controls) = failing_memory_store().await; | ||
| let base_uri = "memory:///"; | ||
| let schema = create_test_schema(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
"memory:///" should be "memory://".
Extra trailing slash deviates from the canonical form specified by the guideline.
As per coding guidelines: "Use plain \"memory://\" URIs in tests; no atomic counters or unique suffixes are needed."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/src/dataset/mem_wal/write.rs` around lines 4897 - 4900, Update the
base_uri value in
test_durable_watcher_does_not_alias_across_memtable_generations from the
noncanonical "memory:///" form to the plain "memory://" form, preserving the
rest of the test unchanged.
Source: Coding guidelines
Doc-comment-only follow-up to the request-scoped durability fix: - State the ordering invariant at the `ShardWriter::put` lock block: the accepting BatchStore/indexes must be captured BEFORE the `maybe_trigger_*` calls that can freeze the active MemTable, or the request-scoped flush completion binds to the wrong (new) generation. - Document the `BatchDurabilityState` variants' behavioral semantics, noting that `Watermark` is cross-generation and unsafe for durable MemTable writes (lance-format#7760), while `FlushCompletion` is request-scoped. - Cross-link `BatchDurableWatcher::wait` and `is_durable`. - Document the lib regression test's failure mode so the assertion ties back to the cross-generation aliasing bug. No behavioral change. cargo fmt, cargo clippy -p lance --tests -D warnings, and the full mem_wal lib suite (515 tests) pass. Co-Authored-By: Claude <noreply@anthropic.com>
ba772ed to
e6ce2d5
Compare
Summary
Fixes #7760.
BatchStorerange that accepted the write.WriteResultpositions, WAL-only behavior, bindings, and persistent formats.Root cause
MemTable batch positions restart at zero after generation rotation, while the previous durability watcher compared those positions against a writer-global watermark. A successful flush from generation N could therefore satisfy a durable write at the same local position in generation N+1 before that write's WAL append completed.
Failure sequence
sequenceDiagram autonumber participant C as Client participant G0 as Writer A / generation N participant M as Shared durability watermark participant G1 as Writer A / generation N+1 participant B as Writer B participant W as WAL C->>G0: durable put, local range 0..1 G0->>W: flush generation N W-->>G0: success G0->>M: publish watermark 1 Note over G0,G1: MemTable rotates and local positions restart at 0 B->>W: claim a higher writer epoch C->>G1: durable put, local range 0..1 G1->>M: wait for watermark >= 1 M-->>G1: already satisfied by generation N G1-->>C: incorrect success G1->>W: flush generation N+1 W-->>G1: PeerClaimedEpochFix
BatchStoreand indexes while holding the writer lock and before a size-triggered freeze can replace the active MemTable.BatchDurableWatcherwith that same cell.Validation
cargo test -p lance --test integration_tests mem_wal::durable_put_does_not_alias_across_memtable_generations -- --exact— passed (1/1)cargo test -p lance dataset::mem_wal::wal::tests --lib— passed (20/20)cargo test -p lance dataset::mem_wal::write::tests --lib— passed (53/53)cargo fmt --all -- --check— passedpre-commit run --files rust/lance/src/dataset/mem_wal/wal.rs rust/lance/src/dataset/mem_wal/write.rs rust/lance/tests/integration_tests.rs rust/lance/tests/mem_wal.rs— passedcargo clippy --all --tests --benches -- -D warnings— passedcargo clippy --all-features --tests --benches -- -D warnings— passedCompatibility
Maintainer note
Please apply the
critical-fixlabel: the bug could report a durable write as successful before its own WAL flush completed.Summary by CodeRabbit