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-699. Upgrade ZooKeeper version to 3.9 #496

Merged
merged 6 commits into from
Feb 15, 2024
Merged

Conversation

tisonkun
Copy link
Member

No description provided.

Signed-off-by: tison <wander4096@gmail.com>
ByteBuffer duplicate = si.request.duplicate();
duplicate.rewind();
ByteBufferInputStream.byteBuffer2Record(duplicate, createRequest);
CreateRequest createRequest = si.readRequestRecord(CreateRequest::new);
Copy link
Member Author

Choose a reason for hiding this comment

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

This may cause failures on early ZK versions and we need a testing compatible layer.

Let the CI verify if it's the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. It fails.

Let me think if we should drop support <= 3.7 or add a test compatible switch.

cc @kezhuw @eolivelli @cammckenzie @Randgalt

@eolivelli
Copy link
Contributor

I think that it is fine to drop support for 3.7 in those tests

@tisonkun
Copy link
Member Author

tisonkun commented Feb 1, 2024

Make sense. I understand it as introducing a new test tag said zk38OrLater to turn on related tests only when zk 3.8+ as dep.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

[ERROR] Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.44 s <<< FAILURE! - in org.apache.curator.framework.imps.TestTransactionsOld
[ERROR] org.apache.curator.framework.imps.TestTransactionsOld.testWithCompression  Time elapsed: 0.144 s  <<< ERROR!
java.lang.NoSuchMethodError: 'com.google.common.base.Predicate org.apache.curator.framework.api.transaction.CuratorTransactionResult.ofTypeAndPath(org.apache.curator.framework.api.transaction.OperationType, java.lang.String)'
	at org.apache.curator.framework.imps.TestTransactionsOld.testWithCompression(TestTransactionsOld.java:165)

It seems because we relocate these methods in curator-framework but not for the test-jar. I don't find a way to relocate the test jar now ..

@tisonkun
Copy link
Member Author

tisonkun commented Feb 14, 2024

I create a monkey patch to work around it - c44e798

Signed-off-by: tison <wander4096@gmail.com>
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

tisonkun commented Feb 14, 2024

@eolivelli Thanks for your review. I make this patch work now. Let me create the ticket and a few follow-ups to improve the case (It now takes 2 hours to complete, I believe we can drop a few versions, but better with a few separate tickets).

@tisonkun tisonkun changed the title CURATOR-XXX. Upgrade ZooKeeper version to 3.9 CURATOR-699. Upgrade ZooKeeper version to 3.9 Feb 15, 2024
@tisonkun tisonkun merged commit 972fffa into apache:master Feb 15, 2024
6 checks passed
@tisonkun tisonkun deleted the zk39 branch February 15, 2024 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants