xiangfu0 opened a new pull request, #20018: URL: https://github.com/apache/pinot/pull/20018
## Problem `RealtimeSegmentValidationManager` calls `PinotLLCRealtimeSegmentManager.repairSegmentsInErrorState` on every run. For a committed (non-`IN_PROGRESS`) segment with at least one replica in `ERROR`, the repair called `PinotHelixResourceManager.resetSegment(table, segment, null)`. A `null` target instance expands to every instance of the segment in the ideal state, and `resetPartitionAllState` resets a replica regardless of its current state, so the healthy `ONLINE` replicas were bounced through `OFFLINE` together with the broken one. A segment with a single bad replica therefore lost all of its replicas for the duration of the reset. Two more problems in the same pass: - `resetSegment` throws when any instance fails to reset and the loop had no handling for it, so the first failure aborted the repair of the whole table (`RuntimeException: Reset segment failed for table: ...`). - A failed reset or re-ingestion was only logged; there was no metric to alert on. ### Production evidence (StarTree Cloud, DATA-3674) - splunk/startree-us1, 2026-09-27: `rum_otel_data__33__6702` had one replica of three in `ERROR`. The repair at 01:03:56 UTC reset all three and partition 33 had zero `ONLINE` replicas for about 1 s. - rvtech/prod-rvt, 2026-10-09: a disk-full restart left about 5,700 committed remote-tier segments with one `ERROR` copy each. The repair reset both copies of every segment, each healthy copy went `OFFLINE` for 14 to 17 s, `SegmentStatusChecker` caught segments with zero `ONLINE` replicas and `SegmentReplicasCriticallyLowForHATable` paged. The pass also aborted on the first failed reset, so it cleared only about 92 segments per 15-minute run. ## Fix - The external-view scan now records which instances hold an `ERROR` replica for each affected segment. For non-`IN_PROGRESS` segments the repair resets only those instances, one `resetSegment(table, segment, instance, true)` call each, so the `ONLINE` replicas keep serving throughout. - `PinotHelixResourceManager.resetSegment` gets an `errorReplicasOnly` overload that re-reads the external view and resets a replica only if it is still in `ERROR` at that moment. A replica that recovered on its own while the repair was walking a large table (the rvtech pass took hours) is therefore not bounced either. The existing three-argument method keeps its behaviour (reset everything that is not `OFFLINE`), which the reset REST API relies on. - The all-replicas-in-`ERROR` reset for `IN_PROGRESS` segments and the re-ingestion path for `COMMITTING` segments are unchanged. - Every reset and re-ingestion attempt is wrapped individually. A failure is logged, counted in the new table-level meter `ControllerMeter.LLC_ERROR_SEGMENT_REPAIR_FAILURES` (exported as `llcErrorSegmentRepairFailures`), and the pass moves on to the next replica or segment. No wire format, ZK schema, or config key changes, so mixed-version clusters are unaffected. ## Tests `PinotLLCRealtimeSegmentManagerTest`: - `testRepairSegmentsInErrorStateResetsOnlyErrorReplicas`: one `ERROR` replica of three resets only that instance; a committed segment with all replicas in `ERROR` resets each instance; no reset is ever issued with a `null` (all instances) target. - `testRepairSegmentsInErrorStateContinuesAfterFailedReset`: a failing reset on the first segment is metered and the next segment is still reset. - `testRepairSegmentsInErrorStateKeepsInProgressSegmentHandling`: the existing `IN_PROGRESS` handling is preserved (a consuming segment with all replicas in `ERROR` is reset on every instance, a consuming segment with a healthy replica is left alone), and a failure of that all-replicas reset is metered instead of escaping. ## Repair cadence (asked in the ticket) The repair runs on every `RealtimeSegmentValidationManager` run, scheduled by `controller.realtime.segment.validation.frequencyPeriod` (OSS default `1h`, the affected deployments run `15m`); it is not gated by `controller.segment.level.validation.intervalPeriod`. A shorter repair interval is therefore already a config change, with the caveat that each run also executes `ensureAllPartitionsConsuming` for every realtime table (an ideal-state update plus a stream partition-metadata fetch per table). A one-off repair can be triggered with `GET /periodictask/run?taskname=RealtimeSegmentValidationManager&tableName=<table>`. An event-triggered repair (an external-view listener on the controller) is a larger change and is left out of this PR. Note that a consuming (`IN_PROGRESS`) segment with one `ERROR` replica is, as before, not reset by this pass; it is repaired by the first pass after it commits. The splunk segment above had already committed by the time it was repaired, which is why it hit the all-replicas reset. ## SegmentStatusChecker and ERROR replicas (asked in the ticket) Yes, the exclusion is deliberate. `SegmentStatusChecker` skips a segment entirely (for every per-segment gauge, not only the `ERROR` count) when its ZK metadata znode was modified within `controller.statusChecker.waitForPushTimeInSeconds` (default `10m`) before the external-view snapshot. The code comment explains that this avoids false alerts while servers load a freshly created, committed, or pushed segment. It also skips segments that have no `ONLINE`/`CONSUMING` replica in the ideal state. There is no exclusion based on a segment being in `CONSUMING` state as such: a consuming segment older than the grace window with an `ERROR` replica is counted. The replica of `rum_otel_data__33__6702` missed at 00:49, 00:54 and 01:00 UTC is consistent with that grace window if the segment's znode was created or updated (commit) within the 10 minutes before each scan; the ticket has no znode mtime, so this is not verified. One related observation, not changed here: `ControllerGauge.SEGMENTS_IN_ERROR_STATE` is written by two tasks with different definitions. `SegmentStatusChecker` sets it to the number of segment-replica pairs in `ERROR` (after the grace window), while `repairSegmentsInErrorState` sets it to the number of segments whose replicas are all in `ERROR` for non-pauseless tables, so the gauge flips between the two values depending on which task ran last. -- 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]
