Jackie-Jiang commented on code in PR #18559:
URL: https://github.com/apache/pinot/pull/18559#discussion_r3284538088
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java:
##########
Review Comment:
Not introduced in this PR, but is this part redundant?
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java:
##########
@@ -135,6 +140,15 @@ public IdealState commit(HelixManager helixManager, String
resourceName, Functio
if (!ent._resourceName.equals(resourceName)) {
continue;
}
+ if (ent._cancelled) {
+ // Cancelled by a previous failed batch's leader (its commit()
catch). Skip and
+ // remove so this batch never applies an updater whose owner
has already given up.
+ // Without this, the owner's catch (e.g. cleanup of a
newly-created segment ZK
+ // metadata in the pauseless commit path) races against this
batch's CAS write
+ // and produces an "in IdealState, no ZK metadata" orphan.
+ it.remove();
+ continue;
+ }
processed.add(ent);
it.remove();
updatedIdealState = ent._updater.apply(updatedIdealState);
Review Comment:
Is this bug triggered by the update logic throws exception? (e.g. some
previous updates cause one update to NPE)?
--
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]
