awslabs / spark-sql-kinesis-connector

Spark Structured Streaming Kinesis Data Streams connector supports both GetRecords and SubscribeToShard (Enhanced Fan-Out, EFO)
Apache License 2.0
26 stars 13 forks source link

Update to pom.xml to prevent java.lang.NoSuchMethodError: java.nio.By… #3

Closed pankajmisra closed 11 months ago

pankajmisra commented 1 year ago

…teBuffer.rewind()Ljava/nio/ByteBuffer

Issue #, if available: The code when compiled and run in emr-notebook environment, fails with the above message during writeStream.

Description of changes: The pom.xml was updated to include maven.compiler.release of 8 in its properties.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

hwanghw commented 11 months ago

@pankajmisra thank you for your PR! Did you build the connector with JDK 9+? I don't think it will have java.lang.NoSuchMethodError, if it is built with JDK8.

I tested your change using JDK8 to build the connector, and got [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project spark-streaming-sql-kinesis-connector_2.12: Fatal error compiling: invalid flag: --release -> [Help 1]

maven.compiler.release can only be activated for JDK9+. Reference: https://stackoverflow.com/questions/59049980/maven-compiler-release-as-an-replacement-for-source-and-target

pankajmisra commented 11 months ago

Yes, I had built it with JDK 11.

I understand that this error would not come with JDK8 build. For JDK8 build, the pom file was good with source and target compiler version tags.

But in order to make the build work for JDK9+, this change was suggested. I also acknowledge and understand that maven.compiler.release would work only for JDK9+, but that is a more common scenario today when compared to the expecting people installing JDK8 specifically to build this code.

The suggested change would help with broader adoption, with the mention of JDK9+ as the required version to build.

With JDK9+ also the current code builds but then it fails at runtime with recent versions of emr as consumer while consuming from Kinesis streams. Hence suggesting adding the maven.compiler.release. This usage would be more common going forward and may help with better adoption of the connector.

hwanghw commented 11 months ago

I understand your point, thank you. I'd suggest to follow the recommendation in the reference link I previously shared so that pom.xml can work for both JDK8 and JDK9+.

<profile>
  <id>java-8-compatible</id>
  <activation>
    <jdk>[9,)</jdk>
  </activation>
  <properties>
    <maven.compiler.release>8</maven.compiler.release>
  </properties>
</profile>
hwanghw commented 11 months ago

I updated the pom file to support both JDK8 and JDK9+ build.

Thank you again for your contribution and pointing out the JDK9+ build problem.