Filter out non-local updates in shard positions update event handler#6613
Filter out non-local updates in shard positions update event handler#6613guilload wants to merge 1 commit into
Conversation
![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: aebe071e92
ℹ️ 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".
| let Some(shard) = state_guard.shards.get(queue_id) else { | ||
| return false; | ||
| }; | ||
| if shard_position.is_eof() { | ||
| state_guard.delete_shard(&queue_id, "indexer gossip").await; | ||
| } else if !shard_position.is_beginning() { | ||
| state_guard | ||
| .truncate_shard(&queue_id, shard_position, "indexer gossip") | ||
| .await; | ||
| return true; |
There was a problem hiding this comment.
When a WAL queue exists without an in-memory shard, this early filter drops the update before delete_shard can run. That matters because delete_shard deletes the WAL queue before checking shards.remove, and init_primary_shard can leave such a dangling queue if it creates the queue and then fails before inserting the shard (the code even has a TODO for that cleanup). For EOF gossip on that local dangling queue, the previous handler would remove the WAL queue, but this path now treats it as non-local and leaves it consuming disk/capacity.
Useful? React with 👍 / 👎.
1 participant
Description
ShardPositionsUpdatecarry all the updates for a source. Most shards contained in the update are probably not relevant to the ingester so:I actually don't expect this PR to solve the issue mentioned in #6598 but I'm hoping that the new spans and logging statement will help us gain a better understanding of what's going on.
How was this PR tested?
c t --manifest-path quickwit/Cargo.toml -p quickwit-ingest