-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Upgrade to Vert.x 4.4.9 with backports #39788
Conversation
I'm having issues locally with native compilation, but this could be due to GraalVM baselines, to be investigated. |
There's indeed a native compilation issue to fix, I've been able to pass native tests fine locally on a recent GraalVM + clean 3.2 branch. |
👋 @zakkak @geoand I'm struggling with this backport, it looks like some Netty classes get build-time initialized while they shouldn't. It's really the Netty version bump that brings these issues on 3.2:
|
I can take a look on Monday |
This comment has been minimized.
This comment has been minimized.
It's picking
|
We can exclude this file, there is a build item (I think the Oracle jdbc extension uses it) |
|
I'll get back to this tomorrow (PTO) - I had tried excluding those metadata but ended up going circles / it looks like some SSL-related code is triggering those build-time initialisations. |
Tracing class initializations always point me to
I haven't seen any obvious change in the Netty / Vert.x extensions between |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
And I assume that initializing at |
@geoand that's what I ended up doing but we still have the gRPC and cache native tests failing, but not on native compilation. I don't know if it's related. |
Indeed it's hard to tell... |
I've been able to test It looks like there is a more serious issue with
I have spotted the characteristic warning where a heuristic fails to handle an
/cc @zakkak it's not related to your efforts wrt |
Hi @alesj, I'd need some help on backporting gRPC changes to 3.2 with a Netty/Vert.x bump. I've backported a substitution so I haven't bumped the Do you have any idea? 🙏 |
@jponge how do I reproduce this compilation errors? |
@alesj I realise I did not answer your question 🤣 Here's what you need to do, aligning the gRPC/protobuf to what's in diff --git a/pom.xml b/pom.xml
index e78e18008e2..7a80d6b53a5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -72,11 +72,11 @@
<kubernetes-client.version>6.7.2</kubernetes-client.version> <!-- Please check with Java Operator SDK team before updating -->
<!-- Make sure to check compatibility between these 2 gRPC components before upgrade -->
- <grpc.version>1.56.0</grpc.version> <!-- when updating, verify if com.google.auth should not be updated too -->
+ <grpc.version>1.62.2</grpc.version> <!-- when updating, verify if com.google.auth should not be updated too -->
<grpc-jprotoc.version>1.2.1</grpc-jprotoc.version>
- <protoc.version>3.22.0</protoc.version>
+ <protoc.version>3.25.0</protoc.version>
<protobuf-java.version>${protoc.version}</protobuf-java.version>
- <proto-google-common-protos.version>2.23.0</proto-google-common-protos.version>
+ <proto-google-common-protos.version>2.36.0</proto-google-common-protos.version>
<!-- TestNG version: we don't enforce it in the BOM as it is mostly used in the MP TCKs and we need to use the version from the TCKs -->
<testng.version>7.4.0</testng.version> |
@zakkak how did you fix |
Resolves issues unveiled with quarkusio#37347
It just worked after the patch. CI seems happy about it as well. Note that this is without the grpc update mentioned in #39788 (comment) |
@zakkak My current branch is the same as this PR (without any gRPC update) and still get issues in native with
I'm on macOS using |
This comment has been minimized.
This comment has been minimized.
@zakkak Ok, so I have an interesting development here:
|
I just relaunched the CI on JVM Tests / JDK 11, the rest is green. @gsmet do you prefer a single commit or is the current set of 4 commits ok? |
There was a report about this in 3.9.x as well, it might not be 3.2.x specific. See https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Native.20compilation.20issue.20with.203.2E9.2E2 Please let me know if we need to make it work with Oracle GraalVM for JDK 17 as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✔️ | JVM Tests - JDK 11 | Failures | Logs | Raw logs | 🚧 | |
✔️ | JVM Tests - JDK 17 | Logs | Raw logs | 🚧 |
Full information is available in the Build summary check run.
Failures
⚙️ JVM Tests - JDK 11 #
- Failing: independent-projects/tools/analytics-common
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/maven and 205 more
📦 independent-projects/tools/analytics-common
✖ io.quarkus.analytics.AnalyticsServiceTest.sendAnalyticsTest
line 160
- History - More details - Source on GitHub
org.awaitility.core.ConditionTimeoutException:
Assertion condition defined as a io.quarkus.analytics.AnalyticsServiceTest Expected at least one request matching: {
"url" : "/v1/track",
"method" : "POST"
}
Requests received: [ ] within 5 seconds.
at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
* If a client sends more than this number of parameters in a request, the connection is closed. | ||
*/ | ||
@ConfigItem(defaultValue = "1000") | ||
public int maxParameters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jponge this config parameter doesn't seem to be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it might be removed
See quarkusio#39819 and quarkusio#39788 (cherry picked from commit b35ae00)
See quarkusio#39819 and quarkusio#39788 (cherry picked from commit b35ae00)
See quarkusio#39819 and quarkusio#39788 (cherry picked from commit b35ae00) (cherry picked from commit d2673c8)
See quarkusio#39819 and quarkusio#39788 (cherry picked from commit b35ae00) (cherry picked from commit d2673c8)
See quarkusio#39819 and quarkusio#39788 (cherry picked from commit b35ae00) (cherry picked from commit d2673c8)
This is a backport of #39764 on top of Vert.x 4.4.9