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

support jdbc connection with unix domain socket in native image #1940

Closed
czp3009 opened this issue Apr 12, 2024 · 8 comments · Fixed by #1961 or #1950
Closed

support jdbc connection with unix domain socket in native image #1940

czp3009 opened this issue Apr 12, 2024 · 8 comments · Fixed by #1961 or #1950
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@czp3009
Copy link

czp3009 commented Apr 12, 2024

Feature Description

I'm sorry I don't really know where this issue should be submit, so I'll submit it here first, and let me know if it shouldn't be.

I've looked at the previous archived repositories, but I don't know what repositories the previous functionality has been moved to.

I've submitted a PR to that repository before: GoogleCloudPlatform/native-image-support-java#304

That PR add unix domain socket supporting in graalvm native images.

But now when I add dependency according to the new documentation, I still get the same error as before this PR

java.lang.ClassNotFoundException: jnr.ffi.provider.jffi.Provider
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.hub.ClassForNameSupport.forName(ClassForNameSupport.java:123) ~[na:na]
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.hub.ClassForNameSupport.forName(ClassForNameSupport.java:87) ~[na:na]
        at java.base@17.0.10/java.lang.Class.forName(DynamicHub.java:1322) ~[report-service:na]
        at java.base@17.0.10/java.lang.Class.forName(DynamicHub.java:1285) ~[report-service:na]
        at java.base@17.0.10/java.lang.Class.forName(DynamicHub.java:1278) ~[report-service:na]
        at jnr.ffi.provider.FFIProvider$SystemProviderSingletonHolder.getInstance(FFIProvider.java:68) ~[na:na]
        at jnr.ffi.provider.FFIProvider$SystemProviderSingletonHolder.<clinit>(FFIProvider.java:57) ~[na:na]
        at jnr.ffi.provider.FFIProvider.getSystemProvider(FFIProvider.java:35) ~[report-service:na]
        at jnr.ffi.Runtime$SingletonHolder.<clinit>(Runtime.java:109) ~[na:na]
        at jnr.ffi.Runtime.getSystemRuntime(Runtime.java:70) ~[report-service:na]
        at jnr.unixsocket.SockAddrUnix.<init>(SockAddrUnix.java:46) ~[report-service:na]
        at jnr.unixsocket.SockAddrUnix$DefaultSockAddrUnix.<init>(SockAddrUnix.java:208) ~[na:na]
        at jnr.unixsocket.SockAddrUnix.create(SockAddrUnix.java:174) ~[report-service:na]
        at jnr.unixsocket.UnixSocketAddress.<init>(UnixSocketAddress.java:47) ~[na:na]
        at com.google.cloud.sql.core.Connector.connect(Connector.java:106) ~[na:na]
        at com.google.cloud.sql.core.InternalConnectorRegistry.connect(InternalConnectorRegistry.java:179) ~[na:na]
        at com.google.cloud.sql.mysql.SocketFactory.connect(SocketFactory.java:63) ~[report-service:1.16.0]
        at com.google.cloud.sql.mysql.SocketFactory.connect(SocketFactory.java:45) ~[report-service:1.16.0]

I think that repository was using an older version of the code before moving it to another repository, so the code in the last batch of PRs didn't get moved to the new repository.

If this functionality should be moved to this repository, is it possible to merge the code from the previous PR to this repository?

Sample code

No response

Alternatives Considered

No response

Additional Details

No response

@czp3009 czp3009 added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Apr 12, 2024
@jackwotherspoon
Copy link
Collaborator

@czp3009 Thanks for raising an issue on the Cloud SQL Java Connector! Going to hand this over to our resident Java export to take a look at 😄

@hessjcg mind taking a look at this?

@czp3009
Copy link
Author

czp3009 commented Apr 12, 2024

After reading the source code, I'm sure that https://github.com/GoogleCloudPlatform/native-image-support-java/blob/master/native-image-support/src/main/java/com/google/cloud/nativeimage/features/gcp/CloudSqlFeature.java in the old repository has been moved to https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory/blob/main/core/src/main/java/com/google/cloud/sql/nativeimage/CloudSqlFeature.java in this repository.

But not include last PR in old repository. Which results in lost the support for unix domain sockets in the graalvm native image.

If you guys manage both repositories, please merge the last PR into this one.

By the way, I see that there's another issue: #217

The 'connecting directly' in that issue is means unix domain socket in graalvm native image. The last PR already solved this problem in the old repository, if merge back into this repository can also solve that issue.

@hessjcg
Copy link
Collaborator

hessjcg commented Apr 12, 2024

@czp3009 Thanks for the heads up. I'll port GoogleCloudPlatform/native-image-support-java#304 over to this repo.

@hessjcg
Copy link
Collaborator

hessjcg commented Apr 15, 2024

A quick status update: it seems like we can't port this over exactly 1:1. Perhaps there are different versions of graalvm installed. I'm looking into it. I would welcome help from someone more experienced with graalvm.

@jackwotherspoon jackwotherspoon added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 16, 2024
@hessjcg
Copy link
Collaborator

hessjcg commented Apr 19, 2024

PR #1947 is a dead end. It won't work because we need to be able to compile the source code using JDK 8, but the configuration required to add the proxy. I'm going to try again replacing core/src/main/java/com/google/cloud/sql/nativeimage/CloudSqlFeature.java with a JSON file containing the Graalvm reflection configuration.

@czp3009
Copy link
Author

czp3009 commented Apr 20, 2024

hi @hessjcg , i think it's a good idea to register the proxy with a JSON file, instead of replacing the entire CloudSqlFeature with JSON.

for example(proxy-config.json):

[
  {
    "interfaces": [
      "jnr.unixsocket.Native$LibC",
      "jnr.ffi.provider.LoadedLibrary"
    ]
  },
  {
    "interfaces": [
      "jnr.enxio.channels.Native$LibC",
      "jnr.ffi.provider.LoadedLibrary"
    ]
  }
]

@hessjcg
Copy link
Collaborator

hessjcg commented Apr 24, 2024

Ok, we are getting closer. The classes now load, but it seems like the resources JNI library resources are not being loaded correctly. When I run the test, I get this error: (See #1961)

java.lang.UnsatisfiedLinkError: could not get native definition for type `POINTER`, original error message follows: java.lang.UnsatisfiedLinkError: could not locate stub library in jar file.  Tried [jni/x86_64-Linux/libjffi-1.2.so, /jni/x86_64-Linux/libjffi-1.2.so]

I think that the nativeimage resource configuration is not correct yet.

@hessjcg
Copy link
Collaborator

hessjcg commented Apr 30, 2024

Code is complete. These PRs should fix the issue:

#1960
#1961

hessjcg added a commit that referenced this issue May 1, 2024
This is a fix for the graalvm configuration issues related to the jnr.unixsocket library.

The proxy classes are now being properly configured for reflection and dynamic proxy.
Native library files that are usually loaded as resources from jffi-1.3.13-native.jar are included in the image.

Fixes #1940
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
3 participants