[ https://issues.apache.org/jira/browse/IGNITE-8131?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16547576#comment-16547576 ]
Denis Garus edited comment on IGNITE-8131 at 7/18/18 9:30 AM: -------------------------------------------------------------- [~sergey-chugunov] If we'll add field ZkRuntimeState.topVer, we must use it everywhere rather then ZkDiscoveryEventsData#topVer (about 20 usages), else it would be confusing. This would be the reason some refactorings that touched plenty code base. If we'll choose this way, I think, we should assign to ZkRuntimeState.topVer default value 1L and add follow condition into ZookeeperDiscoveryImpl#processNewEvents(byte[]): {code:java} if (rtState.joined) rtState.evtsData = newEvts; else rtState.topVer = 1L; //may be it will be constant {code} I would like offer to move the assigning of rtState.evtsData to ZookeeperDiscoveryImpl#processNewEvents(ZkDiscoveryEventsData) before onEventProcessed call, like this: {code:java} if (rtState.joined) rtState.evtsData = evtsData; if (rtState.crd) handleProcessedEvents("procEvt"); else onEventProcessed(rtState, updateNodeInfo, evtProcessed); {code} It looks like small change and also fix the issue. What do you think? was (Author: garus.d.g): [~sergey-chugunov] If we'll add field ZkRuntimeState.topVer, we must use it everywhere rather then ZkDiscoveryEventsData#topVer (about 20 usages), else it would be confusing. This would be the reason some refactorings that touched plenty code base. If we'll choose this way, I think, we should assign to ZkRuntimeState.topVer default value 1L and add follow condition into ZookeeperDiscoveryImpl#processNewEvents(byte[]): {code:java} if (rtState.joined) rtState.evtsData = newEvts; else rtState.topVer = 1L; //may be it will be constant {code} I would like offer to replace the assigning of rtState.evtsData to ZookeeperDiscoveryImpl#processNewEvents(ZkDiscoveryEventsData) before onEventProcessed call, like this: {code:java} if (rtState.joined) rtState.evtsData = evtsData; if (rtState.crd) handleProcessedEvents("procEvt"); else onEventProcessed(rtState, updateNodeInfo, evtProcessed); {code} It looks like small change and also fix the issue. What do you think? > ZookeeperDiscoverySpiTest#testClientReconnectSessionExpire* tests fail on TC > ---------------------------------------------------------------------------- > > Key: IGNITE-8131 > URL: https://issues.apache.org/jira/browse/IGNITE-8131 > Project: Ignite > Issue Type: Bug > Components: zookeeper > Reporter: Sergey Chugunov > Assignee: Denis Garus > Priority: Major > Labels: MakeTeamcityGreenAgain > Fix For: 2.7 > > Attachments: ZK_client_reconnect_failure.log, > ZK_client_reconnect_success.log > > > Two tests always fail on TC with the assertion > {noformat} > junit.framework.AssertionFailedError: Failed to wait for disconnect/reconnect > event. > at > org.apache.ignite.spi.discovery.zk.internal.ZookeeperDiscoverySpiTest.waitReconnectEvent(ZookeeperDiscoverySpiTest.java:4221) > at > org.apache.ignite.spi.discovery.zk.internal.ZookeeperDiscoverySpiTest.reconnectClientNodes(ZookeeperDiscoverySpiTest.java:4183) > at > org.apache.ignite.spi.discovery.zk.internal.ZookeeperDiscoverySpiTest.clientReconnectSessionExpire(ZookeeperDiscoverySpiTest.java:2231) > at > org.apache.ignite.spi.discovery.zk.internal.ZookeeperDiscoverySpiTest.testClientReconnectSessionExpire1_1(ZookeeperDiscoverySpiTest.java:2206) > {noformat} > from client disconnect/reconnect events check. Obviously client doesn't > generate these events as it supposed to do. > (TC runs can be found > [here|https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_IgniteZooKeeperDiscovery&branch_IgniteTests24Java8=pull%2F3730%2Fhead&tab=buildTypeStatusDiv]). > It is possible to reproduce test failure locally as well, but with low > probability: one failure for 50 or even 300 successful executions. -- This message was sent by Atlassian JIRA (v7.6.3#76005)