Add decommission metrics - #6646
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: 07d719c0d4
ℹ️ 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".
loutPhilipps
left a comment
loutPhilipps
left a comment
There was a problem hiding this comment.
LGTM, not sure logging for compactor decommisionning errors is critical but won't hurt to have it!
ec8a228 to
fe2cba8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe2cba8a4d
ℹ️ 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".
| .map(|queue_summary| { | ||
| queue_summary | ||
| .end | ||
| .map(|end| end.saturating_sub(queue_summary.start) + 1) |
There was a problem hiding this comment.
When a queue has been fully truncated, mrecordlog keeps the last position while the queue is empty (the existing state test documents this as “queue is now empty but remembers position 1”), so end < start is a valid empty range. With saturating_sub(...)+1, that case is reported as one remaining record, causing decommission timeout diagnostics and the new wal_num_records observation field to overstate WAL contents whenever an ingester is waiting on shard deletion after all records were truncated; handle end < start as zero instead.
Useful? React with 👍 / 👎.
| let self_node_id = self_node_id.clone(); | ||
| let mrecordlog = mrecordlog.clone(); | ||
| async move { | ||
| let mrecordlog_guard = mrecordlog.read().await; |
There was a problem hiding this comment.
The readiness loop in quickwit-serve/src/lib.rs calls try_get_ingester_status without a timeout, but the first observation now awaits the WAL read lock before yielding the status. If WAL initialization/opening or a long append/truncate holds the write lock, readiness reporting stalls at this await and keeps the previous chitchat/health state instead of sampling the ingester status; consider emitting the status immediately or making WAL stats optional/non-blocking for status-only callers.
Useful? React with 👍 / 👎.
2 participants
Description
Add decommission metrics and log clearly what is the outcome of a decommissioning attempt.
How was this PR tested?