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

SNOW-917458: Snowflake JDBC drivers fails on JDK 21 with Arrow serialization #1512

Closed
wendigo opened this issue Sep 18, 2023 · 31 comments · Fixed by #1529
Closed

SNOW-917458: Snowflake JDBC drivers fails on JDK 21 with Arrow serialization #1512

wendigo opened this issue Sep 18, 2023 · 31 comments · Fixed by #1529
Assignees
Labels

Comments

@wendigo
Copy link

wendigo commented Sep 18, 2023

The root cause of the problem is the fact that snowflake jdbc driver is using Arrow version before 13.0.0 which includes a fix for the DirectByteBuffer constructor change, introduced in JDK 21: apache/arrow#36370

The solution is to either manually backport this change to a driver or upgrade Arrow to 13.0.0

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

  1. What version of JDBC driver are you using?

Latest

  1. What operating system and processor architecture are you using?

Linux/MacOS, both arm and intel

  1. What version of Java are you using?

JDK 21-ea (build 35)

  1. What did you do?

Query snowflake using JDBC driver. It fails when Arrow serialization is used.

  1. What did you expect to see?

Working driver

  1. Can you set logging to DEBUG and collect the logs?

    https://community.snowflake.com/s/article/How-to-generate-log-file-on-Snowflake-connectors

  2. What is your Snowflake account identifier, if any? (Optional)

@wendigo wendigo added the bug label Sep 18, 2023
@github-actions github-actions bot changed the title Snowflake JDBC fails on JDK 21 SNOW-917458: Snowflake JDBC fails on JDK 21 Sep 18, 2023
@wendigo wendigo changed the title SNOW-917458: Snowflake JDBC fails on JDK 21 Snowflake JDBC drivers fails on JDK 21 with Arrow serialization Sep 18, 2023
@turcsanyip
Copy link

I ran into the same issue described by @wendigo.
Any chance to backport apache/arrow#36370 in the near future?

@sfc-gh-spanaite sfc-gh-spanaite changed the title Snowflake JDBC drivers fails on JDK 21 with Arrow serialization SNOW-917458: Snowflake JDBC drivers fails on JDK 21 with Arrow serialization Oct 3, 2023
@sfc-gh-spanaite sfc-gh-spanaite self-assigned this Oct 3, 2023
@sfc-gh-spanaite
Copy link
Contributor

@wendigo Can you post the exception you are seeing?

@sfc-gh-spanaite sfc-gh-spanaite added the status-triage Issue is under initial triage label Oct 3, 2023
@wendigo
Copy link
Author

wendigo commented Oct 3, 2023

java.lang.UnsupportedOperationException: sun.misc.Unsafe or java.nio.DirectByteBuffer.<init>(long, int) not available
       org.apache.arrow.memory.util.MemoryUtil.directBuffer(MemoryUtil.java:167)
       org.apache.arrow.memory.ArrowBuf.getDirectBuffer(ArrowBuf.java:228)
       org.apache.arrow.memory.ArrowBuf.nioBuffer(ArrowBuf.java:223)
       org.apache.arrow.vector.ipc.ReadChannel.readFully(ReadChannel.java:87)
       org.apache.arrow.vector.ipc.message.MessageSerializer.readMessageBody(MessageSerializer.java:727)
       org.apache.arrow.vector.ipc.message.MessageChannelReader.readNext(MessageChannelReader.java:67)
       org.apache.arrow.vector.ipc.ArrowStreamReader.loadNextBatch(ArrowStreamReader.java:145)

The relevant Arrow change is linked in the issue description.

@wendigo
Copy link
Author

wendigo commented Oct 3, 2023

This is due to the change in the DirectByteBuffer constructor in JDK 21 (previously it was (long, int), now it is (long, long)): openjdk/jdk@a56598f

@sfc-gh-spanaite sfc-gh-spanaite removed the status-triage Issue is under initial triage label Oct 4, 2023
@sfc-gh-spanaite
Copy link
Contributor

I'm checking internally if we can backport the Arrow fix.

@tpoll
Copy link

tpoll commented Oct 12, 2023

Any updates here? I'm running into the exact same issue and it's currently the only item blocking our upgrade to JDK 21.

@sfc-gh-spanaite
Copy link
Contributor

@tpoll We're still looking into this, but I don't have yet any timelines. It might take some time since we have some other priorities as of now, but I'll get back with an update as soon as I have more specific information.

@sfc-gh-wfateem
Copy link
Collaborator

@tpoll @wendigo would you be able to build PR #1529 and confirm that helps resolve your issue? I tested this locally and it works for me. It would be great if you could test it for your particular workload just to confirm everything else is functioning as expected since we're making a leap in the Arrow library release version from 10.0.0 to 13.0.0 to fix this.

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

Sure @sfc-gh-wfateem ! I can confirm that in a couple of minutes

@tpoll
Copy link

tpoll commented Oct 13, 2023

@sfc-gh-wfateem thanks for the quick turn around. I just tested the PR against several of our workloads and confirmed the patch seems to resolve the issue. I didn't observe any other regression.

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

@sfc-gh-wfateem @tpoll I'm on 50/284 tests, and so far, so good :)

@sfc-gh-wfateem
Copy link
Collaborator

Now that I'm thinking about this. I'm wondering if this is going to break support for JDK 8.
What versions of the JDK are you testing with @tpoll @wendigo?

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

Why do you think that it will break 8 @sfc-gh-wfateem ?

@sfc-gh-wfateem
Copy link
Collaborator

We had to upgrade Jackson to 2.15.1 and I saw that it introduced a multi-release JAR, which in return we're shading. I'm not sure if a JDK 8 runtime environment will understand what to do in that situation or not.
I just tested it with JDK 8 and there's an ugly crash:

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x0000000106c83279, pid=14015, tid=0x0000000000001003
#
# JRE version: OpenJDK Runtime Environment (8.0_372) (build 1.8.0_372-bre_2023_04_25_03_16-b00)
# Java VM: OpenJDK 64-Bit Server VM (25.372-b00 mixed mode bsd-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.dylib+0x52e279]  Unsafe_SetBoolean+0x61
#

I'll need to look into it a bit more.

@tpoll
Copy link

tpoll commented Oct 13, 2023

I tested on JDK 17 and JDK 21

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

Seems like this one: netty/netty#12119

@sfc-gh-wfateem
Copy link
Collaborator

@wendigo yup. That looks like it.

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

Homebrew/homebrew-core#106195 similiar one

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

From my experience, I found the Temurin builds the most stable in comparison to OpenJDK/Zulu/Corretto

@sfc-gh-wfateem
Copy link
Collaborator

@wendigo it looks like the issue is rectified on JDK 8 after I removed the following argument:

 -XX:StartFlightRecording=name=background,maxsize=200m,filename=jfr_data.jfr

Still, we wouldn't be able to merge and release this without considering it a breaking change. We would need to discuss how to move forward with this.

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

@sfc-gh-wfateem have you tried to use other vendor than OpenJDK?

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

I did run most of the test and everything is fine with this change on JDK 21 (Temurin, 21-35):

Screenshot 2023-10-13 at 21 34 18

@sfc-gh-wfateem
Copy link
Collaborator

@wendigo No I haven't, but we have a lot of customers that use OpenJDK so we're looking at it from that perspective. It's irrelevant if it works on other distributions and it's not going to be feasible to test every vendor's JDK 8 distribution.

@wendigo
Copy link
Author

wendigo commented Oct 13, 2023

I'm surprised that anyone is using, non-TCK attested distribution in production :) Brave!

@tpoll
Copy link

tpoll commented Oct 20, 2023

Any updates here? Some potential routes could be shading the fix into the old version of arrow or else trying to do a multi release jar.

@jtjeferreira
Copy link
Contributor

If the snowflake jar had a non-shaded distribution, users could upgrade dependencies by themselves...

@wendigo
Copy link
Author

wendigo commented Oct 31, 2023

Or you could release per jdk driver using maven classifier like mssql does (https://mvnrepository.com/artifact/com.microsoft.sqlserver/mssql-jdbc)

@wendigo
Copy link
Author

wendigo commented Dec 4, 2023

@sfc-gh-wfateem is there a release date for this change?

@sfc-gh-wfateem
Copy link
Collaborator

Hey @wendigo,
The fix is going to be included in the JDBC driver version 3.14.4, and that should be out roughly in two weeks time, unless that has to be delayed for any reason.

@wendigo
Copy link
Author

wendigo commented Dec 7, 2023

@sfc-gh-wfateem new version was released today afaik

@sfc-gh-wfateem
Copy link
Collaborator

@wendigo yes, it looks like 3.14.4 was released sooner than I anticipated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants