suvodeep-pyne commented on code in PR #19909:
URL: https://github.com/apache/pinot/pull/19909#discussion_r4339386415
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -3036,6 +3036,24 @@ public void repairSegmentsInErrorState(TableConfig
tableConfig,
}
}
+ /// Resets only the replicas of the segment that are in ERROR state in the
external view. Healthy replicas are left
+ /// untouched: resetting a healthy replica takes it through OFFLINE, which
on dedup / upsert tables briefly drops its
+ /// keys from the metadata manager while other replicas keep consuming, and
can make the only good copy of a
+ /// segment without a deep-store copy unavailable.
+ private void resetErrorReplicas(String realtimeTableName, String
segmentName, Map<String, String> currentStateMap) {
+ for (Map.Entry<String, String> entry : currentStateMap.entrySet()) {
+ if (SegmentStateModel.ERROR.equals(entry.getValue())) {
+ String instanceName = entry.getKey();
+ try {
+ _helixResourceManager.resetSegment(realtimeTableName, segmentName,
instanceName);
Review Comment:
Agreed. Fixed in a779ff97dd, beyond the fresh ExternalView check:
- The repair collects the segments to reset and, after the loop, makes one
call to the new `PinotHelixResourceManager.resetErrorReplicas(table,
segments)`. That method reads the IdealState and ExternalView when it sends the
resets, and only targets replicas in ERROR there.
- The ExternalView lags the instance's current state, so the per-instance
reset (`resetPartitions(..., errorStateOnly=true)`) also skips segments that
are no longer ERROR in the current state it reads.
- Each message is sent with `fromState=ERROR`, as
`HelixAdmin.resetPartition` does. If the replica leaves ERROR before the server
processes the message, Helix rejects the message
(`HelixStateMismatchException`) and leaves the current state alone.
So a replica that has recovered is never taken through OFFLINE. Covered by
`SegmentResetTest.testResetErrorReplicasResetsOnlyReplicasStillInError` and
`testResetPartitionsErrorStateOnlySendsResetsFromErrorForPartitionsInError`.
--
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]
