rohityadav1993 opened a new pull request, #19310: URL: https://github.com/apache/pinot/pull/19310
## Summary `missingConsumingSegmentTotalCount` and its two sibling gauges report a false positive for REALTIME tables whose ingestion is intentionally paused. This gates the check on the table's pause state and resets the gauges while paused. ## Problem `SegmentStatusChecker.updateSegmentMetrics` invokes `MissingConsumingSegmentFinder` for every enabled REALTIME table, with no check for whether the table is paused via the pause/resume ingestion API. When a table is paused, `PinotLLCRealtimeSegmentManager` deliberately does not create a replacement CONSUMING segment after the current one commits, leaving the partition with only a completed segment. `MissingConsumingSegmentFinder` treats exactly that shape — no consuming segment, a completed segment, and a stream offset that has advanced past it — as a missing consumer. Since the stream keeps advancing while paused, the condition never clears, so `MISSING_CONSUMING_SEGMENT_MAX_DURATION_MINUTES` grows for the duration of the pause. The result is indistinguishable from a genuine ingestion failure. Disabled tables are unaffected: `updateSegmentMetrics` returns early on `!idealState.isEnabled()` and calls `removeMetricsForTable`. Paused tables had no equivalent guard. The existing per-topic `PauseState.getIndexOfInactiveTopics()` exclusion inside the finder does not cover this. `updatePauseStateInIdealState` only carries forward a pre-existing inactive-topic list; a whole-table pause never populates it, so `isPaused() == true` leaves the finder's per-topic exclusion empty. ## Fix Reuse the pause state already computed in `updateSegmentMetrics` for the `TABLE_CONSUMPTION_PAUSED` gauge, and skip the finder when the table is paused. These gauges are last-write-wins with no "unset" value, so simply skipping the call would leave whatever value was written before the pause in place, alerting indefinitely. The paused branch therefore actively resets them through a new `MissingConsumingSegmentFinder.resetMetrics`, which keeps the set of gauge names in the class that emits them — otherwise a future gauge added to `findAndEmitMetrics` would silently go stale on the paused path, reintroducing this same bug. No config, REST, metric-name, or wire-format changes. The only new surface is `MissingConsumingSegmentFinder.resetMetrics`, an internal helper on an existing controller-side class. ### Known gap / possible follow-up This gates on whole-table pause (`isTablePaused`) only. Individually paused topics in a multi-topic stream config (`PauseState.getIndexOfInactiveTopics()`, added in #16692) are already handled for the ordinary case: `PartitionGroupMetadataFetcher.fetchMultipleStreams` skips paused topic indices, so those partitions never enter `_partitionGroupIdToLargestStreamOffsetMap` and the main detection loop never examines them. There is a narrower gap, which I have left alone here. When that map ends up empty, `findMissingSegments` falls back to iterating `partitionGroupIdToLatestCompletedSegmentMap` and counts every partition without a consuming segment as missing, consulting neither pause state nor stream offsets. Pausing *every* topic of a multi-topic table individually reaches that fallback. The same fallback is also taken when the stream-metadata fetch throws, which is existing behaviour and a separate question. That case seemed better handled on its own, since it is about the fallback path's semantics rather than the pause gate this PR adds. Happy to follow up separately, or to fold it in here if reviewers prefer. ## Test Plan New `SegmentStatusCheckerTest.realtimePausedTableHasNoMissingConsumingSegmentAlert`, built on the existing `realtimeBasicTest` fixture and run in two phases against the same metrics instance: 1. unpaused — asserts `MISSING_CONSUMING_SEGMENT_TOTAL_COUNT == 2`, establishing a non-zero reading 2. paused — asserts all three `MISSING_CONSUMING_SEGMENT_*` gauges are `0` The two-phase shape is deliberate: it covers the actual production scenario, where the gauge is already reporting non-zero from an earlier run and the pause must actively clear it, rather than merely never setting it. Verified the test fails without the fix. With `SegmentStatusChecker.java` reverted to master, the run fails on the second phase with `expected [0] but found [2]`, and the surefire log contains no `Caught exception while updating segment status` — confirming the failure is a real computed value rather than a swallowed exception leaving the gauge at its default. - `SegmentStatusCheckerTest`: 42/42 pass - `MissingConsumingSegmentFinderTest`: 6/6 pass, no regression - `spotless:apply`, `checkstyle:check`, `license:check` on `pinot-controller`: clean -- 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]
