Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CURATOR-588 Upgrade ZooKeeper to 3.6.3 #386

Merged
merged 3 commits into from Jul 6, 2021
Merged

Conversation

eolivelli
Copy link
Contributor

It looks like upgrading to ZK 3.6.3 is not that easy as for 3.6.2

pom.xml Outdated Show resolved Hide resolved
@virajjasani
Copy link

Thanks @eolivelli.

Build failure:

The command "eval mvn install -DskipTests=true -Dmaven.javadoc.skip=true -B -V " failed

Is this relevant or some other known issue?

@eolivelli
Copy link
Contributor Author

eolivelli commented Jul 2, 2021

This patch is still a draft, because some tests are still failing

There is some behaviour changes in ZK 3.6.3 and we have to understand all the failures

@eolivelli
Copy link
Contributor Author

@cammckenzie I have fixed the problem for zk35 compatibility test.

The weird behaviour is about a ConnectionLoss vs SessionExpired that happens in a test from 3.6.3 onwards

@eolivelli eolivelli changed the title Upgrade ZK to 3.6.3 CURATOR-588 Upgrade ZooKeeper to 3.6.2 Jul 5, 2021
@eolivelli eolivelli changed the title CURATOR-588 Upgrade ZooKeeper to 3.6.2 CURATOR-588 Upgrade ZooKeeper to 3.6.3 Jul 5, 2021
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for starting this upgrade! Comment inline.

@@ -670,9 +670,13 @@ public void testFailure() throws Exception
client.checkExists().forPath("/hey");
fail();
}
catch ( KeeperException.SessionExpiredException e )
{
// correct, this happens on ZK 3.6.3+
Copy link
Member

@tisonkun tisonkun Jul 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Shall we add a link to the reason?

It is a bit weird a patch version bump cause behavior changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really do not know why this is happening.
I have looked thru ZK 3.6.3 release notes and I haven't found anything that may be releated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine. It is not a blocker for client library such as Curator, but is interesting...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably Session Expiration is happening earlier with 3.6.3
with 3.6.2 we still see a ConnectionLoss, that usually becomes a SessionExpired when the client is able to connect again and the ZK server already expired it on the server side

Copy link
Contributor

@cammckenzie cammckenzie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, all tests pass.

@eolivelli eolivelli merged commit e42a07a into apache:master Jul 6, 2021
@eolivelli eolivelli deleted the fix/zk363 branch July 6, 2021 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants