ingest: batch WAL truncation in the shard-positions gossip handler#6599
ingest: batch WAL truncation in the shard-positions gossip handler#6599aanufriev-celonis wants to merge 1 commit into
Conversation
aanufriev-celonis
force-pushed
the
wal-cleanup-lock-hold-ingest-stall
branch
from
f6d235b to
fc7ed60
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7ed6001b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| // Drop the guard (end of scope) and yield so a runnable persist/init_shards task can | ||
| // be polled and take the lock before re-acquire for the next batch. | ||
| tokio::task::yield_now().await; |
There was a problem hiding this comment.
When a gossip update spans multiple batches, this yield_now().await still runs while state_guard is in scope, so the full ingester/WAL lock is held during the yield. A queued persist or init_shards task that gets polled by this yield can only block on the same lock, and the lock is not actually released until this handler is scheduled again and reaches the end of the loop body; this adds idle lock-hold time and undermines the intended interleaving between batches. Explicitly drop the guard before yielding or put the guarded work in an inner scope.
Useful? React with 👍 / 👎.
aanufriev-celonis
force-pushed
the
wal-cleanup-lock-hold-ingest-stall
branch
from
fc7ed60 to
fcfe740
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Fixes #6598
Problem
See the original description of #6598.
Fix
Process the update in small batches (
GOSSIP_TRUNCATE_BATCH_SIZE = 8), acquiring and releasing the lock per batch instead of once for the whole update. Between batches the lock is free, so queuedpersist/init_shardscalls interleave.This is turn-processing on the shared WAL lock, not added parallelism: the WAL is a single shared log, and its operations remain serialized. The change only bounds how long the cleanup task holds the lock before yielding it.
Compatibility
The per-shard helpers already re-validate state on each call, so releasing and re-acquiring the lock mid-update cannot corrupt state:
truncate_shardre-looks up the shard and only advances the truncation position;delete_shardtreats an already-missing WAL queue as success.Testing
test_ingester_truncate_on_shard_positions_update_spans_multiple_batches, which drives more shards than a single batch (mixed truncate + EOF-delete) and asserts every shard is processed correctly across the multiple lock acquisitions.