mcvsubbu commented on a change in pull request #6483:
URL: https://github.com/apache/incubator-pinot/pull/6483#discussion_r563999016
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -793,6 +793,27 @@ void
updateInstanceStatesForNewConsumingSegment(Map<String, Map<String, String>>
LOGGER.info("Updating segment: {} to ONLINE state",
committingSegmentName);
}
+ // There used to be a race condition in pinot (caused by heavy GC on the
controller during segment commit)
Review comment:
Last time this happened, it was caused by gc on the controller, and the
same controller waking up to complete the operation it is scheduled to do.
I could not dig up the PR since it happened (I think) when the source code
was under com.linkedin domain, but it was fixed by @jackjlli by adding a Stat
check when we update metadata for the committing segment.
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -793,6 +793,27 @@ void
updateInstanceStatesForNewConsumingSegment(Map<String, Map<String, String>>
LOGGER.info("Updating segment: {} to ONLINE state",
committingSegmentName);
}
+ // There used to be a race condition in pinot (caused by heavy GC on the
controller during segment commit)
+ // that ended up creating multiple consuming segments for the same stream
partition, named somewhat like
+ // tableName__1__25__20210920T190005Z and
tableName__1__25__20210920T190007Z. It was fixed by checking the
+ // Zookeeper Stat object before updating the segment metadata.
+ // These conditions can happen again due to manual operations considered
as fixes in Issues #5559 and #5263
+ // The following check prevents the table from going into such a state
(but does not prevent the root cause
+ // of attempting such a zk update).
+ if (instanceStatesMap != null) {
Review comment:
Removed null check
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -793,6 +793,27 @@ void
updateInstanceStatesForNewConsumingSegment(Map<String, Map<String, String>>
LOGGER.info("Updating segment: {} to ONLINE state",
committingSegmentName);
}
+ // There used to be a race condition in pinot (caused by heavy GC on the
controller during segment commit)
+ // that ended up creating multiple consuming segments for the same stream
partition, named somewhat like
+ // tableName__1__25__20210920T190005Z and
tableName__1__25__20210920T190007Z. It was fixed by checking the
+ // Zookeeper Stat object before updating the segment metadata.
+ // These conditions can happen again due to manual operations considered
as fixes in Issues #5559 and #5263
+ // The following check prevents the table from going into such a state
(but does not prevent the root cause
+ // of attempting such a zk update).
+ if (instanceStatesMap != null) {
Review comment:
Helix returns an empty TreeMap() when we create new table
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -793,6 +793,27 @@ void
updateInstanceStatesForNewConsumingSegment(Map<String, Map<String, String>>
LOGGER.info("Updating segment: {} to ONLINE state",
committingSegmentName);
}
+ // There used to be a race condition in pinot (caused by heavy GC on the
controller during segment commit)
Review comment:
In any case, it picks up the latest Idealstate and checks on that znode.
If a different controller added a segment, then even if the old controller gets
back control, it will not be able to add the new segment
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
