You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/22 22:14:36 UTC
[GitHub] [kafka] rondagostino opened a new pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
rondagostino opened a new pull request #10918:
URL: https://github.com/apache/kafka/pull/10918
Updates ZooKeeper to v3.6.3. Adds some additional exercising of ZooKeeper in the upgrade system tests. Also adds 2.8.0 to the system test docker image.
The dependency on `io.dropwizard.metrics:metrics-core` cannot be avoided as per https://issues.apache.org/jira/browse/ZOOKEEPER-4324. All integration tests based on `ZooKeeperTestHarness` fail without the dropwizard metrics jar being on the CLASSPATH; system tests fail as well.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#issuecomment-871627589
Thanks, @rondagostino . Since the only change was to a comment, I will not wait for Jenkins to re-run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663417535
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
https://github.com/apache/zookeeper/commit/13fe0d0ffb9fd2c379b9b430aaaf9ee75acfceba
It's not the only version update that was made, but no code changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r656620373
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
Does this conflict with the yammer metrics library we use in Kafka?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663412933
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
Why are we using this old version? The latest version is 4.2.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663416886
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
The straightforward way would be to check the version bump in the 3.7 branch, did that require code changes?
CVEs are the common reason to have to update these days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r656620095
##########
File path: docs/upgrade.html
##########
@@ -21,6 +21,13 @@
<h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3.0.0</a></h5>
<ul>
+ <li>ZooKeeper has been upgraded to 3.6.3, and that version has a hard dependency on the
+ <code>io.dropwizard.metrics:metrics-core:3.2.5</code> library due to the new metrics subsystem added in 3.6.0.
+ Setting <code>metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider</code> in your
+ zookeeper.properties file does not remove this dependency. ZooKeeper will fail to start with
+ <code>java.lang.NoClassDefFoundError: com/codahale/metrics/Reservoir</code> if you do not have the above library
+ on your CLASSPATH.
+ </li>
Review comment:
Isn't this handled automatically for users?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#issuecomment-868693454
I've reverted the change that made the `io.dropwizard.metrics:metrics-core` library a test-only dependency due to packaging concerns -- would it be included in the release, for example?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r664827028
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
https://github.com/apache/kafka/pull/10982 contains this change. Will attache system test results there when tests complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663413353
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
https://github.com/apache/zookeeper/blob/release-3.6.3/pom.xml#L367
```
<dropwizard.version>3.2.5</dropwizard.version>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #10918:
URL: https://github.com/apache/kafka/pull/10918
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r659527329
##########
File path: tests/docker/Dockerfile
##########
@@ -62,6 +62,7 @@ RUN mkdir -p "/opt/kafka-2.4.1" && chmod a+rw /opt/kafka-2.4.1 && curl -s "$KAFK
RUN mkdir -p "/opt/kafka-2.5.1" && chmod a+rw /opt/kafka-2.5.1 && curl -s "$KAFKA_MIRROR/kafka_2.12-2.5.1.tgz" | tar xz --strip-components=1 -C "/opt/kafka-2.5.1"
RUN mkdir -p "/opt/kafka-2.6.2" && chmod a+rw /opt/kafka-2.6.2 && curl -s "$KAFKA_MIRROR/kafka_2.12-2.6.2.tgz" | tar xz --strip-components=1 -C "/opt/kafka-2.6.2"
RUN mkdir -p "/opt/kafka-2.7.1" && chmod a+rw /opt/kafka-2.7.1 && curl -s "$KAFKA_MIRROR/kafka_2.12-2.7.1.tgz" | tar xz --strip-components=1 -C "/opt/kafka-2.7.1"
+RUN mkdir -p "/opt/kafka-2.8.0" && chmod a+rw /opt/kafka-2.8.0 && curl -s "$KAFKA_MIRROR/kafka_2.12-2.8.0.tgz" | tar xz --strip-components=1 -C "/opt/kafka-2.8.0"
Review comment:
Thanks for adding this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663415801
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
> have we checked if we have to stick with this old version
No. How would we do that check? I can think of 2 ways: 1) Feels too risky: just take the new version and call it good if the system tests pass; 2) Ask in the Zk mailing list maybe? Any other options?
> It will be harder to update in the 3.0 series after that's released
It feels to me like the straightforward thing to do is use the version that ZK uses, and there's no need to update the dropwizard version unless we update the ZK version. And if we are sticking with ZK 3.6.x through the EOL of ZooKeeper with Kafka (unknown, but potentially the case) then there's no reason to change it sans some kind of CVE. Maybe I'm missing something, though?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r661689038
##########
File path: build.gradle
##########
@@ -823,6 +823,7 @@ project(':core') {
implementation libs.scalaLogging
implementation libs.slf4jApi
implementation(libs.zookeeper) {
+ implementation libs.dropwizardMetrics
Review comment:
Good point -- added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r663414434
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
3.7 uses a newer version though, have we checked if we have to stick with this old version? It will be harder to update in the 3.0 series after that's released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r657107216
##########
File path: gradle/dependencies.gradle
##########
@@ -61,6 +61,7 @@ versions += [
bcpkix: "1.66",
checkstyle: "8.36.2",
commonsCli: "1.4",
+ dropwizardMetrics: "3.2.5",
Review comment:
> Does this conflict with the yammer metrics library
It doesn't, no. The yammer metrics library is called `metrics-core-2.2.0.jar` and has all classes underneath the `com.yammer.metrics` package. The dropwizard library is called `metrics-core-3.2.5.jar` and has all classes underneath the `com.codahale.metrics` package. It isn't ideal that the jar names differ only by the version, but there is no conflict.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r661676599
##########
File path: build.gradle
##########
@@ -823,6 +823,7 @@ project(':core') {
implementation libs.scalaLogging
implementation libs.slf4jApi
implementation(libs.zookeeper) {
+ implementation libs.dropwizardMetrics
Review comment:
Can you add a comment explaining that this is needed for Apache ZooKeeper, and should NOT be used by Kafka itself?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#issuecomment-867111180
System test results before making the jar test dependency: https://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2021-06-23--001.1624469608--rondagostino--KAFKA-12756--bbc88d7e8/report.html
```
================================================================================
SESSION REPORT (ALL TESTS)
ducktape version: 0.8.1
session_id: 2021-06-23--001
run time: 175 minutes 4.253 seconds
tests run: 867
passed: 659
failed: 11
ignored: 197
================================================================================
```
- `kafkatest.tests.core.zookeeper_authorizer_test` failed for `metadata_quorum:REMOTE_KRAFT` -- unrelated to this change snce ZooKeeper isn't even involved.
- `kafkatest.tests.core.upgrade_test` failed all 3 tests from version 2.8.0; this was due to 2.8.0 not being part of the Vagrant image. I added a commit to this PR to fix that and confirmed that the non-compression flavor of that test for 2.8.0 passed locally with Vagrant (this PR already had the change to add it to the Docker image for Docker-based system tests).
- `kafkatest.sanity_checks.test_kafka_version` failed due to `kafka-topics --zookeeper` being used against the current version rather than the 0.8.2 version; added a commit to this PR to swap the node versions so the correct broker will get the request and confirmed locally that the test now passes.
- `kafkatest.tests.streams.streams_eos_test` had 4 test failures.
- `kafkatest.tests.streams.streams_upgrade_test` had 2 test failures
I believe the steams failures are unrelated because I downgraded to Zookeeper v3.5.9 locally and ran `kafkatest.tests.streams.streams_upgrade_test.StreamsUpgradeTest.test_version_probing_upgrade` -- it still failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rondagostino commented on a change in pull request #10918: KAFKA-12756: Update ZooKeeper to v3.6.3
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10918:
URL: https://github.com/apache/kafka/pull/10918#discussion_r657104600
##########
File path: docs/upgrade.html
##########
@@ -21,6 +21,13 @@
<h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3.0.0</a></h5>
<ul>
+ <li>ZooKeeper has been upgraded to 3.6.3, and that version has a hard dependency on the
+ <code>io.dropwizard.metrics:metrics-core:3.2.5</code> library due to the new metrics subsystem added in 3.6.0.
+ Setting <code>metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider</code> in your
+ zookeeper.properties file does not remove this dependency. ZooKeeper will fail to start with
+ <code>java.lang.NoClassDefFoundError: com/codahale/metrics/Reservoir</code> if you do not have the above library
+ on your CLASSPATH.
+ </li>
Review comment:
> Isn't this handled automatically for users
Yeah, good point, no need to discuss it here. Will remove.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org