pkuwm commented on a change in pull request #904: Fix the concurrent modification error happens during the HelixManager initHandlers() call URL: https://github.com/apache/helix/pull/904#discussion_r395941462
########## File path: helix-core/src/test/java/org/apache/helix/manager/zk/TestHandleSession.java ########## @@ -466,6 +468,79 @@ public void testSessionExpiredWhenResetHandlers() throws Exception { deleteCluster(clusterName); } + @Test + public void testConcurrentInitCallbackHandlers() throws Exception { + final String clusterName = + CLUSTER_PREFIX + "_" + _className + "_" + TestHelper.getTestMethodName(); + TestHelper.setupEmptyCluster(_gZkClient, clusterName); + final String spectatorName = "testConcurrentHandlerChangeSpectator"; + try { + BlockingHandleNewSessionZkHelixManager helixManager = + new BlockingHandleNewSessionZkHelixManager(clusterName, spectatorName, + InstanceType.SPECTATOR, _gZkClient.getServers()); + helixManager.connect(); + // Add two mock listeners that will add more callback handlers while handling INIT or CALLBACK event. + // Note that we have to test with 2 separate listeners so one of them has a chance to fail if + // there is a concurrent modification exception. + helixManager.addLiveInstanceChangeListener( + (LiveInstanceChangeListener) (liveInstances, changeContext) -> { + if (changeContext.getType() != NotificationContext.Type.FINALIZE) { + for (LiveInstance liveInstance : liveInstances) { + if (liveInstance.getInstanceName().equals("localhost_1")) { + try { + helixManager.addCurrentStateChangeListener( + (CurrentStateChangeListener) (instanceName, statesInfo, currentStateChangeContext) -> { + // empty callback + }, liveInstance.getInstanceName(), liveInstance.getEphemeralOwner()); + } catch (Exception e) { + throw new HelixException( + "Unexpected exception in the testConcurrentHandlerProcessing.", e); + } + } + } + } Review comment: Can we wrap this block into a private method to avoid the duplicate code in these 2 listeners? ---------------------------------------------------------------- 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 With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org