Mailing-List: contact dev-help@zookeeper.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@zookeeper.apache.org
From: afine <git@git.apache.org>
To: dev@zookeeper.apache.org
Reply-To: dev@zookeeper.apache.org
References: <git-pr-229-zookeeper@git.apache.org>
In-Reply-To: <git-pr-229-zookeeper@git.apache.org>
Subject: [GitHub] zookeeper pull request #229: ZOOKEEPER-2759: Flaky test: org.apache.zookeepe...
Content-Type: text/plain
Message-Id: <20170428020841.DFC66E01BC@git1-us-west.apache.org>
Date: Fri, 28 Apr 2017 02:08:41 +0000 (UTC)
archived-at: Fri, 28 Apr 2017 02:08:44 -0000

GitHub user afine reopened a pull request:

    https://github.com/apache/zookeeper/pull/229

    ZOOKEEPER-2759: Flaky test: org.apache.zookeeper.server.quorum.QuorumCnxManagerTest.testNoAuthLearnerConnectToAuthRequiredServerWithHigherSid

    I noticed some inconsistent test results from `testNoAuthLearnerConnectToAuthRequiredServerWithHigherSid`. It can be seen failing on Apache infra here: http://mail-archives.apache.org/mod_mbox/zookeeper-dev/201701.mbox/%3c374175863.2852.1485127554310.JavaMail.jenkins@crius%3e
    
    The problem can be "reproduced" by adding a `Thread.sleep(4000)` as the first line of the finally block   for `RecvWorker.run` in `QuorumCnxManager.java`.
    
    The issue is caused by a race condition between the removal of the relevant `sid` from `senderWorkerMap` for `peer0` and the 3 second delay from `assertEventuallyNotConnected`. 
    
    Other tests in this class do not experience the same problem because `testNoAuthLearnerConnectToAuthRequiredServerWithHigherSid` is unique in that it is the only test using `assertEventuallyNotConnected` where the peer with the lower `sid` is not using SASL. This means that the peer with the lower `sid` will always create send and receive workers in `handleConnection` (as it will not throw an exception on `authServer.authenticate(sock, din)`) and may not destroy them in time for the test assertion.
    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/afine/zookeeper ZOOKEEPER-2759

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/229.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #229
    
----
commit c567c494d116b6893e24e5b1b812a6ff24791614
Author: Abraham Fine <afine@apache.org>
Date:   2017-04-17T22:51:27Z

    ZOOKEEPER-2759: Flaky test:
    org.apache.zookeeper.server.quorum.QuorumCnxManagerTest.testNoAuthLearnerConnectToAuthRequiredServerWithHigherSid

commit 112a70ba14f5cbb94e301702b328acfff54b0b02
Author: Abraham Fine <afine@apache.org>
Date:   2017-04-27T03:56:01Z

    fix tests with mockito

commit 55869fac9d0f99eb52156c889e1b5a76424047f6
Author: Abraham Fine <afine@apache.org>
Date:   2017-04-27T20:05:00Z

    add some comments

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---