Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/229
Thanks for work on this! I hope we have more contributors focus on
improving the test.
The proposed fix masks the real problem - it will always success no matter
what - even if the connection was succeeded, since the halt will clean up the
sender worker map of the peer 0.
>> a race condition between the removal of the relevant sid from
senderWorkerMap for peer0 and the 3 second delay from
assertEventuallyNotConnected.
I think there is a race, but my observation on it is slightly different.
The race is between when peer 0 received (and finished) establish connection
with peer 1, and the assert inside assertEventuallyNotConnected.
>> may not destroy them in time for the test assertion.
Because of the race, once peer 0 has received the connection, its sender
worker map will has sid 1 entry forever until the death of the peer 0 itself.
I think instead of checking on peer 0, we can check peer 1. Its sender
worker map should always be clean. We can't check peer 0 here because the
current API does not encode the true state of the connection - it only encodes
what happened (the connection from 1 to 0 was established on 0's side), but it
does not encode the status of the connection (is the connection still alive or
not - in this case peer 1 will close the socket later when it found out peer 0
has no auth. but on peer 0 side the sender work will have sid 1 inside until
its death, what's done has been done.). The state of peer 1 wrt its sender map
should also reflect state of peer 0 since there is only a single connection
between 0 and 1.
Something like this should serve the purpose:
assertEventuallyNotConnected(peer1, 0);
Maybe add a sleep before it to make sure all the connection attempts were
made (from 0 to 1 and from 1 to 0 two ways), but it's a best effort. 5 sec
sounds enough.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
