feat: Add policy excluded intervals to the accumulated compaction status information for reporting - #19975
Conversation
…formation for reporting
github-actions
Bot
added
Area - Documentation
Area - Web Console
Area - Ingestion
labels
kfaraz
left a comment
kfaraz
left a comment
There was a problem hiding this comment.
Thanks for the changes, @capistrant !
While the change itself makes sense, I have suggested an alternative that allows capturing more info.
| return state == State.SKIPPED; | ||
| } | ||
|
|
||
| public boolean isPolicyExcluded() |
There was a problem hiding this comment.
| public boolean isPolicyExcluded() | |
| public boolean isExcludedByPolicy() |
| * {@link CompactionCandidateSearchPolicy}. Unlike {@link #skipped}, such an | ||
| * interval becomes compactible again if the policy thresholds are relaxed. | ||
| */ | ||
| public static CompactionStatus policyExcluded(String reasonFormat, Object... args) |
There was a problem hiding this comment.
| public static CompactionStatus policyExcluded(String reasonFormat, Object... args) | |
| public static CompactionStatus excludedByPolicy(String reasonFormat, Object... args) |
| if (!eligibility.isEligible()) { | ||
| params.getSnapshotBuilder().addToPolicyExcluded( | ||
| candidate.withCurrentStatus( | ||
| CompactionStatus.policyExcluded("Rejected by search policy: %s", eligibility.getReason()) |
There was a problem hiding this comment.
We can omit the prefixed message Rejected by search policy since the state POLICY_EXCLUDED already implies that.
| @JsonProperty | ||
| private final long segmentCountSkipped; | ||
| @JsonProperty | ||
| private final long segmentCountPolicyExcluded; |
There was a problem hiding this comment.
Nit: Might be nicer to have these fields and methods read as xyzExcludedByPolicy.
| public enum State | ||
| { | ||
| COMPLETE, PENDING, RUNNING, SKIPPED | ||
| COMPLETE, PENDING, RUNNING, SKIPPED, POLICY_EXCLUDED |
There was a problem hiding this comment.
I originally wanted SKIPPED to cover all skip reasons, and have the reason message itself be the distinguishing factor between things like excluded by policy, interval locked by another task, interval lies in skip offset.
Over time, we might want to be able to identify the interval/segment/byte counts against all of these reasons.
How do you feel about the following instead:
- Do not add another value to the
Stateenum - Have a new enum or maybe a bunch of constant Strings (aka error codes) which will serve as the skip reason
- Have the compaction snapshot maintain counts for different skip reasons instead and just include that in the report.
- Emit the skip reason as a dimension against the
segment/skipCompact/bytesand other metrics
thanks for the thoughts. I'm on board with keeping everything in skipped. Will prep the changes in the next day or so |
2 participants
Description
#18802 added a compaction policy that prioritizes the most fragmented intervals for compaction. This policy also adds minimum fragmentation thresholds that allow an operator to willingly choose to skip compaction on intervals who do not have certain amounts of uncompacted volume (by uncompacted segment count and by aggregate uncompacted bytes).
These policy excluded intervals were not being reported in the compaction progress when using compaction supervisors, making it hard to understand overall compaction state for datasources when using this policy. This PR aims to close that gap by explicitly reporting on policy excluded intervals in the compaction stats reporting summary. That way datasource owners and/or cluster operators can see if compaction policy thresholds are preventing any of their data from reaching the configured compaction state for their interval.
New Metrics
|
segment/policyExcluded/bytes|Total bytes of this datasource that do not match the auto compaction config but whose intervals were filtered out by the compaction candidate search policy. Relaxing the policy thresholds makes these intervals eligible for compaction again.|dataSource|Varies||
segment/policyExcluded/count|Total number of segments of this datasource that do not match the auto compaction config but whose intervals were filtered out by the compaction candidate search policy.|dataSource|Varies||
interval/policyExcluded/count|Total number of intervals of this datasource that need compaction but were filtered out by the compaction candidate search policy.|dataSource|Varies|Updated web console visualization
These new statistics are folded into the web console reporting for compaction progress
Release note
Properly report statistics on intervals who require compaction to be compliant with their datasources configured compaction state, but are being excluded by the configured cluster compaction policy (for example, intervals with under 500 segments and require compaction but the most fragmented first policy is blocking compaction by having a threshold of 1000 uncompacted segments before executing a compaction run). This gives a true compaction progress report to cluster operators and datasource owners.
Key changed/added classes in this PR
CompactionStatusCompactionConfigBasedJobTemplateThis PR has: