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

Kafka compressions Snappy and LZ4 not supported with native builds #2718

Closed
solsson opened this issue Jun 4, 2019 · 20 comments · Fixed by #15174
Closed

Kafka compressions Snappy and LZ4 not supported with native builds #2718

solsson opened this issue Jun 4, 2019 · 20 comments · Fixed by #15174
Labels
Milestone

Comments

@solsson
Copy link

solsson commented Jun 4, 2019

This limitation is documented in #977 but not in https://quarkus.io/guides/kafka-guide. My question is what level of effort went in to trying to support snappy and gzip? Is JNI with native somehow ruled out, or might it be worth a try after #2412?

Describe the bug

Native builds disable support for message compression types Snappy and Gzip. Any attempt to consume or produce will result in an exception where the compression type is reported as an integer. The enumeration is in https://github.com/apache/kafka/blob/2.2.1/clients/src/main/java/org/apache/kafka/common/record/CompressionType.java.

A review comment in #977 documents the "hack" in https://github.com/quarkusio/quarkus/blob/0.16/extensions/kafka-client/runtime/src/main/java/io/quarkus/kafka/client/runtime/graal/SubstituteSnappy.java.

Expected behavior

Kafka consumers typically handle compression transparently. Compression is per message, not per topic. The broker enumerates supported compressions. Also messages are immutable and AFAIK there is no other way to remove compression than to rewrite the topic.

Actual behavior

For example when trying to consume a message compressed with Snappy:

java.lang.IllegalArgumentException: Unknown or unsupported compression type id: 2
	at org.apache.kafka.common.record.CompressionType.forId(CompressionType.java:61)
	at org.apache.kafka.common.record.DefaultRecordBatch.compressionType(DefaultRecordBatch.java:211)

I haven't tested ZStandard yet. It isn't explicitly disabled, but I think it too requires a native lib.

To Reproduce

  1. Run https://quarkus.io/guides/kafka-guide
  2. Produce using a non-quarkus kafka client with snappy compression.
  3. The aforementioned exception occurs

Environment (please complete the following information):

  • Output of uname -a or ver: I've only run in docker.
  • Output of java -version: 8 and 11
  • GraalVM version (if different from Java): 1.0 rc15 and rc16 tested
  • Quarkus version or git rev: 0.14 through 0.16 tested

Workaround

Use a jvm build. Note that with the FROM in the generated Dockerfile (fabric8/java-alpine-openjdk8-jre) you must add RUN apk add --no-cache libc6-compat in order to avoid the error java.lang.UnsatisfiedLinkError: /tmp/snappy-1.1.7-3d2397f3-516f-45ab-aad1-94be731682f5-libsnappyjava.so: Error loading shared library ld-linux-x86-64.so.2: No such file or directory.

@solsson solsson added the kind/bug Something isn't working label Jun 4, 2019
@solsson
Copy link
Author

solsson commented Jun 19, 2019

There's https://github.com/airlift/aircompressor but I suppose a switch would involve a patch to Kafka client source.

@solsson
Copy link
Author

solsson commented Jun 20, 2019

Can we do something like #2794?

@solsson solsson changed the title Kafka compressions Snappy and GZIP not supported with native builds Kafka compressions Snappy and LZ4 not supported with native builds Jun 20, 2019
@cescoffier
Copy link
Member

It's a bit more tricky as Graal does not support optional "substitutions".

In an ideal world, we would remove Snappy if we can't find it in the classpath. I might be somewhat possible for some of the substitution (methods) but not for all (class substitution).

@solsson
Copy link
Author

solsson commented Jul 10, 2019

Will #3060 help? How about https://github.com/quarkusio/quarkus/projects/10?

In an ideal world, we would remove Snappy if we can't find it in the classpath

Given how Kafka clients fail at runtime with the rather opaque error message described above, what's the rationale for removing the compression types at build time? I suppose an alternative could be to let it fail at runtime with an error that is either class loading or JNI, depending on how the build was produced. IMO the latter provides a way forward, while the current approach rules out any attempt at supporting these compressions.

Edit: to clarify, I'd be happy to experiment with JNI, but first I'd have to un-patch the Kafka client. I asked initially if support has somehow been ruled out? If so the un-patch would never land in the Quarkus code base and thus the experiments with JNI would be a waste of time.

@solsson
Copy link
Author

solsson commented Aug 16, 2019

From #2999 it looks like JNI support is being strengthened. I also noticed that +JNI is default now in native image builds.

@stale
Copy link

stale bot commented Nov 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you!
We are doing this automatically to ensure out-of-date issues does not stay around indefinitely.
If you believe this issue is still relevant please put a comment on it on why and if it truly needs to stay request or add 'pinned' label.

@stale stale bot added the stale label Nov 13, 2019
@maxandersen maxandersen removed the stale label Nov 13, 2019
@gsmet gsmet added the pinned Issue will never be marked as stale label Nov 13, 2019
@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

I think we have optional substitutions now with onlyWith so it might be doable.

@gsmet
Copy link
Member

gsmet commented Jan 21, 2020

@irenakezic this one might be doable now. See my last comment about adding conditions to the substitions depending on what we have in the classpath (there are a few examples in the code). It might be tricky to test though but you know Kafka so that might help :).

@irenakezic
Copy link
Contributor

@gsmet ok thx! I will give it a try but probably not before the weekend.

@irenakezic
Copy link
Contributor

irenakezic commented Feb 7, 2020

@gsmet sorry for taking so long to look into this.
From what I can see there is a problem with MethodHandle in GraalVM (as described here oracle/graal#955) which is used for snappy compression type. I am not sure if I should try to make further substitutions for CompressionType to try to resolve this?
Is the idea to do the following:

  1. If snappy is not on classpath use current implementation for CompressionType supstitution
  2. If snappy is on classpath implement CompressionType substitiutions to avoid MethodHandle.

@marko-mets-gt
Copy link

Is the fix for supporting these compression algorithms doable or not? I also see that ZSTD is also not supported in quarkus.

@cescoffier
Copy link
Member

It's now doable as GraalVM has added optional substitutions.

@spike83
Copy link
Contributor

spike83 commented Feb 13, 2021

I was able to make a workaround.

Therefore I removed https://github.com/quarkusio/quarkus/blob/master/extensions/kafka-client/runtime/src/main/java/io/quarkus/kafka/client/runtime/graal/SubstituteSnappy.java#L26-L65 and builded snapshot

In my project I added the snappy java dependency:

implementation group: 'org.xerial.snappy', name: 'snappy-java', version: '1.1.8.4'

In addition the native library snappyjava needs to be loaded by:

System.loadLibrary("snappyjava");

This native library file libsnappyjava.so has to be in the java.library.path which includes the
/work directory of the docker container (ubi-minimal in my case), but it does not automatically go there.

Extract the snappy-java-x.y.z.jar and add the library by

COPY native/org/xerial/snappy/native/Linux/x86_64/libsnappyjava.so /work/libsnappyjava.so
RUN chmod +x /work/libsnappyjava.so

The class org.xerial.snappy.SnappyInputStream.class has to be registered for reflection.

I would be happy to try to create a pr. But I would need some directions e.g. What would be the right way of adding the native libraries? How should this in general be approached?

Would it be an option to just remove the SubstituteSnappy class? One would get kind of helpful exceptions I think.

@cescoffier
Copy link
Member

Thanks @spike83! That's a great step forward!
I will have a look this week.

Here are a few thoughts:

  1. The native library could be handled by the extension. If you have the snappy dependency, we can add it to the native image instructions.
  2. Once we have a proper integration, we can remove the SubstituteSnappy, but with your work, sounds almost done! We can also have a conditional substitution checking if the snappy classes are on the classpath.

Which version of GraalVM did you use?

@spike83
Copy link
Contributor

spike83 commented Feb 15, 2021

@cescoffier I'm running the build in docker image quay.io/quarkus/ubi-quarkus-native-image:21.0.0-java11 (id: 31ccea2b17ae) so I would say 21.0.

@cescoffier
Copy link
Member

Perfect thanks! MethodHandle support has been added in that version, which explains why you don't need any substitutions.

@mvrk57
Copy link

mvrk57 commented Feb 16, 2021

Hi guys :-) Im trying to use snappy in a native quarkus application too.
Is the only way to get it running to do it the way @spike83 described? Create a snapshot, remove SubstituteSnappy and load the precompiled lib?

Thanks for your help :)

@cescoffier
Copy link
Member

At the moment yes. I will be working on it soon.

@mvrk57
Copy link

mvrk57 commented Feb 17, 2021

thanks a lot! for now I got it working with the workaround. So no need for a hurry ;-)
Have a good day!

@cescoffier
Copy link
Member

#15174 provides an initial support.

The library is embedded in the native image (need to enable snappy and use GraalVM 21+) and loaded when the application starts.

The funny thing is that 4 days ago the MethodHandle used in the Kafka client have been removed... However, it would not enable the support on GraalVM 20.3 as it also needs serialization support.

@cescoffier cescoffier added area/kafka and removed pinned Issue will never be marked as stale triage/gsmet-christmas-list labels Feb 19, 2021
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.

8 participants