suvodeep-pyne commented on code in PR #19909:
URL: https://github.com/apache/pinot/pull/19909#discussion_r4339386768
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -2983,14 +2977,20 @@ public void repairSegmentsInErrorState(TableConfig
tableConfig,
}
Status status = segmentZKMetadata.getStatus();
+ Map<String, String> currentStateMap =
segmentToInstanceCurrentStateMap.get(segmentName);
- // 1) For COMMITTING segments where all replicas are in ERROR, try
re-ingestion (subject to policy)
+ // 1) For COMMITTING segments where all replicas are in ERROR, try
re-ingestion (subject to policy). Re-ingestion
+ // is the only repair that can admit duplicate or inconsistent rows on
dedup / partial upsert tables, so it is
+ // the only repair gated on repairCommittingSegments. The resets below
only reload data another replica or the
+ // deep store already holds, so they stay enabled for such tables.
if (status == Status.COMMITTING &&
segmentsInErrorStateInAllReplicas.contains(segmentName)) {
if (!repairCommittingSegments) {
segmentsInUnRecoverableState += 1;
Review Comment:
Agreed. Fixed in a779ff97dd. The gauge now also counts:
- (a) a committed segment with a peer download URL and no ONLINE or
CONSUMING replica. It is still reset, since the replica may have a local copy,
but it is counted.
- (b) a COMMITTING segment whose re-ingestion finds no server endpoint, or
whose trigger fails.
- (c) a partial-ERROR COMMITTING segment with no ONLINE or CONSUMING
replica. It is still reset and counted (see the reset-loop comment).
- a segment that is still CONSUMING in the IdealState with some replicas in
ERROR but no replica consuming it.
- a segment whose reset fails, or whose reset message isn't written. The
pass then fails (see the swallowed-failures comment).
- on pauseless dedup and partial-upsert tables, an IN_PROGRESS segment with
every replica in ERROR. It is skipped and counted again (see your first
comment).
A segment is counted once even if more than one of these applies. A segment
whose reset is sent but then keeps failing on the server for another reason
still isn't visible to this pass. It stays in
`PAUSELESS_SEGMENTS_IN_ERROR_COUNT`, which I reverted to its master meaning
(segments with every replica in ERROR), for as long as every replica is down.
Writing the gauge after the loop is unchanged from master. An exception from
`getSegmentZKMetadata` propagates out of the pass as it does on master, and
shows in the periodic task's error meter.
--
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]
