FrankChen021 commented on code in PR #19975:
URL: https://github.com/apache/druid/pull/19975#discussion_r3996201167
##########
indexing-service/src/main/java/org/apache/druid/indexing/compact/CompactionConfigBasedJobTemplate.java:
##########
@@ -99,6 +101,15 @@ public List<CompactionJob> createCompactionJobs(
.getCompactionPolicy()
.checkEligibilityForCompaction(candidate,
params.getLatestTaskStatus(candidate));
if (!eligibility.isEligible()) {
+ params.getSnapshotBuilder().addToSkipped(
Review Comment:
[P2] Preserve timeline-stale status before policy filtering
createCompactionJobs checks the search policy before the queue's
CompactionStatusTracker.deriveCompactionStatus check. When the latest
compaction task succeeded after the segment snapshot, the tracker should report
transient TIMELINE_NOT_UPDATED; if the same candidate also fails the policy,
this branch records REJECTED_BY_SEARCH_POLICY as deferred instead. That can
make recently successful work appear not fully compacted until a later refresh.
Derive the task/timeline status before applying policy rejection, or otherwise
preserve the transient status, and add a stale-timeline plus policy-rejection
test.
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionSnapshotBuilder.java:
##########
@@ -101,8 +125,19 @@ private void collectSnapshotStats(AutoCompactionSnapshot
autoCompactionSnapshot)
stats.add(Stats.Compaction.COMPACTED_BYTES, rowKey,
autoCompactionSnapshot.getBytesCompacted());
stats.add(Stats.Compaction.COMPACTED_SEGMENTS, rowKey,
autoCompactionSnapshot.getSegmentCountCompacted());
stats.add(Stats.Compaction.COMPACTED_INTERVALS, rowKey,
autoCompactionSnapshot.getIntervalCountCompacted());
- stats.add(Stats.Compaction.SKIPPED_BYTES, rowKey,
autoCompactionSnapshot.getBytesSkipped());
- stats.add(Stats.Compaction.SKIPPED_SEGMENTS, rowKey,
autoCompactionSnapshot.getSegmentCountSkipped());
- stats.add(Stats.Compaction.SKIPPED_INTERVALS, rowKey,
autoCompactionSnapshot.getIntervalCountSkipped());
+
+ // Skipped stats are emitted per reason. The total for a datasource is the
sum
+ // across all values of the 'reason' dimension. The 'category' dimension
allows
+ // alerting on a class of reasons without enumerating the reasons
themselves.
+ for (CompactionSkipStatistics skipStats :
autoCompactionSnapshot.getSkippedStatsByReason()) {
Review Comment:
[P2] Clear disappeared skip gauge series
collectSnapshotStats emits gauge events only for skip-reason rows present in
the current snapshot. When a reason disappears on the next run, no zero/reset
event is emitted; the default Prometheus exporter retains gauge series unless
its optional flush/TTL is configured, and StatsD gauges likewise retain their
last value. With the new reason/category dimensions, dashboards can therefore
continue to show stale skipped-byte values for a resolved reason. Track
previously emitted label combinations and reset/remove them (or otherwise
guarantee expiry), with a transition test.
##########
web-console/src/druid-models/compaction-status/compaction-status.ts:
##########
@@ -67,7 +146,17 @@ export function formatCompactionInfo(compaction:
CompactionInfo) {
status.intervalCountAwaitingCompaction === 0 &&
!zeroCompactionStatus(status)
) {
- if (status.segmentCountSkipped) {
+ // Intervals skipped for a reason other than being out of scope still
do
+ // not match the compaction config, so the datasource is not fully
compacted
+ const notMatching = skippedStatsNotMatchingConfig(status);
+ if (notMatching.length) {
+ const deferred = skippedStatsOfCategory(status, 'DEFERRED');
+ const reported = deferred.length ? deferred : notMatching;
Review Comment:
[P3] Include all blocking skip categories in status text
skippedStatsNotMatchingConfig(status) can contain multiple blocking
categories, but the message chooses deferred whenever any DEFERRED row exists
and formats only that subset. A status with both DEFERRED and
UNSUPPORTED/PARTIAL rows therefore reports an undercounted Not fully compacted
message even though progress computation sums all nonmatching rows. Format the
complete notMatching set and add a mixed-category UI test.
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionStatusTracker.java:
##########
@@ -97,13 +97,17 @@ public CompactionStatus computeCompactionStatus(
return status;
}
- // Skip intervals that have been filtered out by the policy
+ // Exclude intervals that have been filtered out by the policy
final Eligibility eligibility
= searchPolicy.checkEligibilityForCompaction(candidate,
lastTaskStatus);
if (eligibility.isEligible()) {
return CompactionStatus.pending("Not compacted yet");
} else {
- return CompactionStatus.skipped("Rejected by search policy: %s",
eligibility.getReason());
+ return CompactionStatus.skipped(
Review Comment:
[P2] Classify policy-rejected remainder before pending stats
CompactionStatusTracker now records REJECTED_BY_SEARCH_POLICY when the
iterator entry is inspected, but the legacy coordinator only calls
computeCompactionStatus while task slots remain. Once slots are full,
CompactSegments.updateCompactionSnapshotStats drains the rest with
snapshotBuilder.addToPending(...); policy-rejected candidates in that remainder
are therefore reported as pending and omitted from skippedStatsByReason. Apply
the same status/policy classification while draining the remainder and add a
mixed eligible/rejected regression test.
##########
indexing-service/src/main/java/org/apache/druid/indexing/compact/CompactionConfigBasedJobTemplate.java:
##########
@@ -99,6 +101,15 @@ public List<CompactionJob> createCompactionJobs(
.getCompactionPolicy()
.checkEligibilityForCompaction(candidate,
params.getLatestTaskStatus(candidate));
if (!eligibility.isEligible()) {
+ params.getSnapshotBuilder().addToSkipped(
Review Comment:
[P2] Record iterator skips in supervisor snapshots
CompactionConfigBasedJobTemplate records a skip only for the new
search-policy branch. getCompactibleCandidates still copies
getCompactedSegments() into the snapshot but drops getSkippedSegments(), so an
Overlord/supervisor run whose candidates are all skipped (or has non-policy
skipped candidates alongside jobs) produces no corresponding skip statistics,
and an all-skipped datasource can remain AWAITING_FIRST_RUN. Add every
iterator-level skipped entry to the snapshot and cover offset/size/lock skips
in this path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
