Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.2
    • Labels:
      None

      Description

      The shading rework made before 1.3 removed a snappy dependency that was accidentally pulled in through hadoop. This is technically alright, until class-loaders rear their ugly heads.

      Our kafka connector can read avro records, which may or may not require snappy. Usually this should be solvable by including the snappy dependency in the user-jar if necessary, however since the kafka connector loads classes that it requires using the system class loader this doesn't work.

      As such we have to add a separate snappy dependency to flink-core.

        Issue Links

          Activity

          Hide
          Zentol Chesnay Schepler added a comment -

          1.3: aaa7df55ceb741ca4dd886e8816ddbab58afb078

          Show
          Zentol Chesnay Schepler added a comment - 1.3: aaa7df55ceb741ca4dd886e8816ddbab58afb078
          Hide
          Zentol Chesnay Schepler added a comment -

          I'll merge it today.

          Show
          Zentol Chesnay Schepler added a comment - I'll merge it today.
          Hide
          aljoscha Aljoscha Krettek added a comment -

          Chesnay Schepler what's the plan for merging this to the release-1.3 branch as well and closing this (release blocking) issue?

          Show
          aljoscha Aljoscha Krettek added a comment - Chesnay Schepler what's the plan for merging this to the release-1.3 branch as well and closing this (release blocking) issue?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/4160

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/4160
          Hide
          Zentol Chesnay Schepler added a comment -

          1.4: 021d27d54960bedd58b00e89f7898a34f3991336

          Show
          Zentol Chesnay Schepler added a comment - 1.4: 021d27d54960bedd58b00e89f7898a34f3991336
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/4160

          +1

          Please merge this for `master` and `release-1.3`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4160 +1 Please merge this for `master` and `release-1.3`.
          Hide
          packet Sebastian Klemke added a comment -

          flink-avro connector for DataSet API is also affected: Snappy-encoded avro datasets can neither be read nor written using AvroInputFormat/AvroOutputFormat. Flink runtime built from PR also fixes this.

          Show
          packet Sebastian Klemke added a comment - flink-avro connector for DataSet API is also affected: Snappy-encoded avro datasets can neither be read nor written using AvroInputFormat/AvroOutputFormat. Flink runtime built from PR also fixes this.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/4160#discussion_r123971846

          — Diff: tools/travis_mvn_watchdog.sh —
          @@ -182,6 +182,13 @@ check_shaded_artifacts() {
          exit 1
          fi

          + SNAPPY=`cat allClasses | grep '^org/xerial/snappy' | wc -l`
          + if [ AVRO == "0" ]; then
          — End diff –

          yes! nice catch...

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/4160#discussion_r123971846 — Diff: tools/travis_mvn_watchdog.sh — @@ -182,6 +182,13 @@ check_shaded_artifacts() { exit 1 fi + SNAPPY=`cat allClasses | grep '^org/xerial/snappy' | wc -l` + if [ AVRO == "0" ]; then — End diff – yes! nice catch...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user packet23 commented on a diff in the pull request:

          https://github.com/apache/flink/pull/4160#discussion_r123909436

          — Diff: tools/travis_mvn_watchdog.sh —
          @@ -182,6 +182,13 @@ check_shaded_artifacts() {
          exit 1
          fi

          + SNAPPY=`cat allClasses | grep '^org/xerial/snappy' | wc -l`
          + if [ AVRO == "0" ]; then
          — End diff –

          Should it not read
          ```bash
          if [ $SNAPPY == "0" ]; then

          1. bail out
            fi
            ```
          Show
          githubbot ASF GitHub Bot added a comment - Github user packet23 commented on a diff in the pull request: https://github.com/apache/flink/pull/4160#discussion_r123909436 — Diff: tools/travis_mvn_watchdog.sh — @@ -182,6 +182,13 @@ check_shaded_artifacts() { exit 1 fi + SNAPPY=`cat allClasses | grep '^org/xerial/snappy' | wc -l` + if [ AVRO == "0" ]; then — End diff – Should it not read ```bash if [ $SNAPPY == "0" ]; then bail out fi ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/4160#discussion_r123341447

          — Diff: flink-core/pom.xml —
          @@ -79,13 +79,12 @@ under the License.
          <dependency>
          <groupId>org.apache.avro</groupId>
          <artifactId>avro</artifactId>

          • <!-- managed version -->
          • <exclusions>
          • <exclusion>
          • <groupId>org.xerial.snappy</groupId>
          • <artifactId>snappy-java</artifactId>
          • </exclusion>
          • </exclusions>
            + </dependency>
            +
            + <!-- We explicitly depend on snappy since connectors that require it load it through the system class loader -->
              • End diff –

          for the avro/snappy case, no. This PR should work as is.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/4160#discussion_r123341447 — Diff: flink-core/pom.xml — @@ -79,13 +79,12 @@ under the License. <dependency> <groupId>org.apache.avro</groupId> <artifactId>avro</artifactId> <!-- managed version --> <exclusions> <exclusion> <groupId>org.xerial.snappy</groupId> <artifactId>snappy-java</artifactId> </exclusion> </exclusions> + </dependency> + + <!-- We explicitly depend on snappy since connectors that require it load it through the system class loader --> End diff – for the avro/snappy case, no. This PR should work as is.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on a diff in the pull request:

          https://github.com/apache/flink/pull/4160#discussion_r123298637

          — Diff: flink-core/pom.xml —
          @@ -79,13 +79,12 @@ under the License.
          <dependency>
          <groupId>org.apache.avro</groupId>
          <artifactId>avro</artifactId>

          • <!-- managed version -->
          • <exclusions>
          • <exclusion>
          • <groupId>org.xerial.snappy</groupId>
          • <artifactId>snappy-java</artifactId>
          • </exclusion>
          • </exclusions>
            + </dependency>
            +
            + <!-- We explicitly depend on snappy since connectors that require it load it through the system class loader -->
              • End diff –

          Side question: is a fix also required for connectors to correctly use the user classloader in such cases?

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on a diff in the pull request: https://github.com/apache/flink/pull/4160#discussion_r123298637 — Diff: flink-core/pom.xml — @@ -79,13 +79,12 @@ under the License. <dependency> <groupId>org.apache.avro</groupId> <artifactId>avro</artifactId> <!-- managed version --> <exclusions> <exclusion> <groupId>org.xerial.snappy</groupId> <artifactId>snappy-java</artifactId> </exclusion> </exclusions> + </dependency> + + <!-- We explicitly depend on snappy since connectors that require it load it through the system class loader --> End diff – Side question: is a fix also required for connectors to correctly use the user classloader in such cases?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

          https://github.com/apache/flink/pull/4160

          {FLINK-6965] Include snappy-java in flink-dist

          This PR adds removes the snappy dependency exclusion from flink-core.

          Without this dependency Flink components that work with avro, and thus potentially snappy, fail when loading snappy. This also happens if snappy is provided in the user-jar since the flink component doesn't use the user class loader.

          To prevent this dependency from being removed again in the future by accident i modified `checkShadedArtifacts()` function in the travis scripts to check for the presence of the dependency.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/zentol/flink 6965

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/4160.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #4160


          commit f0e757776c45382cfc28daefe321d819d5a4a75c
          Author: zentol <chesnay@apache.org>
          Date: 2017-06-21T14:18:34Z

          {FLINK-6965] Include snappy-java in flink-dist


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/4160 { FLINK-6965 ] Include snappy-java in flink-dist This PR adds removes the snappy dependency exclusion from flink-core. Without this dependency Flink components that work with avro, and thus potentially snappy, fail when loading snappy. This also happens if snappy is provided in the user-jar since the flink component doesn't use the user class loader. To prevent this dependency from being removed again in the future by accident i modified `checkShadedArtifacts()` function in the travis scripts to check for the presence of the dependency. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 6965 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4160.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4160 commit f0e757776c45382cfc28daefe321d819d5a4a75c Author: zentol <chesnay@apache.org> Date: 2017-06-21T14:18:34Z { FLINK-6965 ] Include snappy-java in flink-dist

            People

            • Assignee:
              Zentol Chesnay Schepler
              Reporter:
              Zentol Chesnay Schepler
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development