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

[fix] Upgrade Vertx to match BK's version (otherwise BK does not work) #20070

Merged
merged 2 commits into from
Apr 12, 2023

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Apr 11, 2023

Motivation

BK deployed with Pulsar fail on start with

ERROR org.apache.bookkeeper.common.component.AbstractLifecycleComponent - Failed to start Component: http-service
java.lang.NoSuchMethodError: 'io.vertx.core.Future io.vertx.core.Vertx.deployVerticle(io.vertx.core.Verticle)'
	at org.apache.bookkeeper.http.vertx.VertxHttpServer.startServer(VertxHttpServer.java:93) ~[org.apache.bookkeeper.http-vertx-http-server-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.server.service.HttpService.doStart(HttpService.java:62) ~[org.apache.bookkeeper-bookkeeper-server-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:83) ~[org.apache.bookkeeper-bookkeeper-common-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.common.component.LifecycleComponentStack.lambda$start$4(LifecycleComponentStack.java:144) ~[org.apache.bookkeeper-bookkeeper-common-4.16.0.jar:4.16.0]
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422) ~[com.google.guava-guava-31.0.1-jre.jar:?]
	at org.apache.bookkeeper.common.component.LifecycleComponentStack.start(LifecycleComponentStack.java:144) ~[org.apache.bookkeeper-bookkeeper-common-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.common.component.ComponentStarter.startComponent(ComponentStarter.java:84) ~[org.apache.bookkeeper-bookkeeper-common-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.server.Main.doMain(Main.java:224) ~[org.apache.bookkeeper-bookkeeper-server-4.16.0.jar:4.16.0]
	at org.apache.bookkeeper.server.Main.main(Main.java:199) ~[org.apache.bookkeeper-bookkeeper-server-4.16.0.jar:4.16.0]

The root cause is that BK upgraded to 4.16.0 and needs newer version of vertx that introduced API changes apache/bookkeeper#3435

Pulsar forces older version of dependency.

Modifications

Upgrade Vertx to the same version as BK

Verifying this change

  • Make sure that the change passes the CI checks.

Update standalone configuration:

diff --git a/conf/standalone.conf b/conf/standalone.conf
index f141946c29f..c7bbe8fdbb6 100644
--- a/conf/standalone.conf
+++ b/conf/standalone.conf
@@ -17,6 +17,11 @@
 # under the License.
 #

+
+httpServerEnabled=true
+httpServerPort=8000
+httpServerClass=org.apache.bookkeeper.http.vertx.VertxHttpServer
+
 ### --- General broker settings --- ###

 # The metadata store URL
@@ -51,7 +56,7 @@ advertisedAddress=
 haProxyProtocolEnabled=false

 # Number of threads to use for Netty IO. Default is set to 2 * Runtime.getRuntime().availableProcessors()
-numIOThreads=
+#numIOThreads=

 # Number of threads to use for ordered executor. The ordered executor is used to operate with zookeeper,
 # such as init zookeeper client, get namespace policies from zookeeper etc. It also used to split bundle. Default is 8

and run bin/pulsar standalone
Without the fix it fails with "Not enough non-faulty bookies available"

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: dlg99#14

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 11, 2023
@cbornet cbornet added this to the 3.0.0 milestone Apr 11, 2023
@cbornet cbornet added the release/blocker Indicate the PR or issue that should block the release until it gets resolved label Apr 11, 2023
@poorbarcode
Copy link
Contributor

/pulsarbot rerun-failure-checks

@RobertIndie RobertIndie merged commit 18ccd9e into apache:master Apr 12, 2023
@tisonkun
Copy link
Member

FWIW - I noticed that the vertx version number is pulled in at #7997 which has a comment:

<!--TODO: When pulsar uses https://github.com/apache/bookkeeper/pull/2410 in -->
<!--      the next bk version, please remove the following content.-->

That is, we temporarily workaround a CVE issue #7931 but later leave it as is. Generally, the vert.x dependency should be conveyed from BK and such issues should be fixed at the BK side.

May or may not we just remove explicit vertx version dependency in Pulsar side to avoid further such version mismatch issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test release/blocker Indicate the PR or issue that should block the release until it gets resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants