Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-7133

Fix Elasticsearch version interference

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.3.0, 1.3.1
    • Fix Version/s: 1.4.0, 1.3.2
    • Component/s: Streaming Connectors
    • Labels:
      None

      Description

      At least two users have encountered problems with shading in the Elasticsearch connector:

      The problem seems to be (quote from the second mail):

      I've found out the source of the problem when I build flink locally.
      elastic-search base depends on (by default) ES version 1.7.1 that depends on
      asm 4.1 and that version is shaded to elasticsearch-base-jar. I tried to set
      elasticsearch.version property in Maven to 5.1.2 (the same as elasticsearch5
      connector) but then elasticsearch-base does not compile:
      
      [ERROR] Failed to execute goal
      org.apache.maven.plugins:maven-compiler-plugin:3.1:testCompile
      (default-testCompile) on project flink-connector-elasticsearch-base_2.11:
      Compilation failure
      [ERROR]
      /home/adebski/Downloads/flink-release-1.3.1/flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/streaming/connectors/elasticsearch/ElasticsearchSinkBaseTest.java:[491,92]
      no suitable constructor found for
      BulkItemResponse(int,java.lang.String,org.elasticsearch.action.ActionResponse)
      [ERROR] constructor
      org.elasticsearch.action.bulk.BulkItemResponse.BulkItemResponse(int,java.lang.String,org.elasticsearch.action.DocWriteResponse)
      is not applicable
      [ERROR] (argument mismatch; org.elasticsearch.action.ActionResponse cannot
      be converted to org.elasticsearch.action.DocWriteResponse)
      [ERROR] constructor
      org.elasticsearch.action.bulk.BulkItemResponse.BulkItemResponse(int,java.lang.String,org.elasticsearch.action.bulk.BulkItemResponse.Failure)
      is not applicable
      [ERROR] (argument mismatch; org.elasticsearch.action.ActionResponse cannot
      be converted to org.elasticsearch.action.bulk.BulkItemResponse.Failure)
      

      To me, it seems like we have to get rid of the "base" package and have two completely separate packages.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/4290
          Hide
          aljoscha Aljoscha Krettek added a comment -

          Fixed on release-1.3 in
          ad8766e10031d813cc0dde82d73d0c8d02f47261

          Fixed on master in
          211d0963e71cac88c18612822956b8312d68a1d7

          Show
          aljoscha Aljoscha Krettek added a comment - Fixed on release-1.3 in ad8766e10031d813cc0dde82d73d0c8d02f47261 Fixed on master in 211d0963e71cac88c18612822956b8312d68a1d7
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on the issue:

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

          Thanks for fixing this @Adebski. 👍 I merged, could you please close this PR?

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/4290 Thanks for fixing this @Adebski. 👍 I merged, could you please close this PR?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          yes

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/4290 yes
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Adebski commented on the issue:

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

          You mean in the pom.xml itself yes?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Adebski commented on the issue: https://github.com/apache/flink/pull/4290 You mean in the pom.xml itself yes?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          Thanks for the PR @Adebski!
          Given that this was tested already for both local and cluster execution,+1 from me once Travis is green.
          Could you also include a comment, preferably referencing the JIRA, on why the dependency is excluded?

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/4290 Thanks for the PR @Adebski! Given that this was tested already for both local and cluster execution,+1 from me once Travis is green. Could you also include a comment, preferably referencing the JIRA, on why the dependency is excluded?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user Adebski opened a pull request:

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

          FLINK-7133Excluding optional asm dependencies from elasticsearch

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

          $ git pull https://github.com/Adebski/flink flink-7133-excluding-asm-from-elastic-search-base

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

          https://github.com/apache/flink/pull/4290.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 #4290


          commit c26f15f8ee10f38b16aa8eb78581980f6359df95
          Author: adebski <andrzej.debski91@gmail.com>
          Date: 2017-07-08T15:03:18Z

          FLINK-7133Excluding optional asm dependencies from elasticsearch artifact so they don't conflict with other shaded asm releases.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user Adebski opened a pull request: https://github.com/apache/flink/pull/4290 FLINK-7133 Excluding optional asm dependencies from elasticsearch You can merge this pull request into a Git repository by running: $ git pull https://github.com/Adebski/flink flink-7133-excluding-asm-from-elastic-search-base Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4290.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 #4290 commit c26f15f8ee10f38b16aa8eb78581980f6359df95 Author: adebski <andrzej.debski91@gmail.com> Date: 2017-07-08T15:03:18Z FLINK-7133 Excluding optional asm dependencies from elasticsearch artifact so they don't conflict with other shaded asm releases.
          Hide
          Zentol Chesnay Schepler added a comment -

          For future reference, if yo want to provide a patch exclusively for a 1.3 branch you should work against release-1.3.

          Show
          Zentol Chesnay Schepler added a comment - For future reference, if yo want to provide a patch exclusively for a 1.3 branch you should work against release-1.3 .
          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment -

          Adebski nice! I'm also in favor of excluding, if it is optional and isn't required in the first place.

          You can just open a PR against the master branch. A committer will apply the patch to the 1.3 release branch so that when releasing 1.3.2 it'll get included. Thanks!

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - Adebski nice! I'm also in favor of excluding, if it is optional and isn't required in the first place. You can just open a PR against the master branch. A committer will apply the patch to the 1.3 release branch so that when releasing 1.3.2 it'll get included. Thanks!
          Hide
          Adebski Adebski added a comment - - edited

          I've changed ES configuration in pom to

          <dependency>
          	<groupId>org.elasticsearch</groupId>
          	<artifactId>elasticsearch</artifactId>
          	<version>${elasticsearch.version}</version>
          	<exclusions>
          		<exclusion>
          			<groupId>org.ow2.asm</groupId>
          			<artifactId>*</artifactId>
          		</exclusion>
          	</exclusions>
          </dependency>
          

          so both asm and asm-commons are excluded (https://mvnrepository.com/artifact/org.elasticsearch/elasticsearch/1.7.1) and everything works fine now - both when executing my simple program from IDEA and when deploying fat jar to local cluster through web interface.

          I can open a PR but I don't know where to put my commit so it is for 1.3.2 version.

          Show
          Adebski Adebski added a comment - - edited I've changed ES configuration in pom to <dependency> <groupId>org.elasticsearch</groupId> <artifactId>elasticsearch</artifactId> <version>${elasticsearch.version}</version> <exclusions> <exclusion> <groupId>org.ow2.asm</groupId> <artifactId>*</artifactId> </exclusion> </exclusions> </dependency> so both asm and asm-commons are excluded ( https://mvnrepository.com/artifact/org.elasticsearch/elasticsearch/1.7.1 ) and everything works fine now - both when executing my simple program from IDEA and when deploying fat jar to local cluster through web interface. I can open a PR but I don't know where to put my commit so it is for 1.3.2 version.
          Hide
          Adebski Adebski added a comment -

          Tzu-Li (Gordon) Tai I didn't, I will try it and report back.

          Chesnay Schepler I think we could but if it turns out it isn't needed excluding it + comment in pom.xml would be IMHO cleaner solution. Especially if guys from ES say themselves that it is an optional dependency.

          Show
          Adebski Adebski added a comment - Tzu-Li (Gordon) Tai I didn't, I will try it and report back. Chesnay Schepler I think we could but if it turns out it isn't needed excluding it + comment in pom.xml would be IMHO cleaner solution. Especially if guys from ES say themselves that it is an optional dependency.
          Hide
          Zentol Chesnay Schepler added a comment -

          Could we not just shade the ES asm dependency to a different namespace?

          Show
          Zentol Chesnay Schepler added a comment - Could we not just shade the ES asm dependency to a different namespace?
          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited

          Adebski Aljoscha Krettek thanks for the explanation

          I assumed that the problem was local only because the last time I tried it with cluster execution for the release testing, it worked without issues.
          I also tried it in local mode just now, however cannot reproduce. I wonder if it may be something specific to the IDE setup, or only surfaces when some other dependency is included.

          I don't really like the idea of removing the `elasticsearch-base` module and duplicating the base classes. Apart from the duplication which isn't nice, if we're solving it this way, then the problem would still exist for the elasticsearch1 module.

          As for another possible resolution, I was checking out this thread: https://github.com/elastic/elasticsearch/issues/7959, and wondering if we can simply exclude the asm dependency from ES.
          From the comments in that thread, it seems like if we're just using ES client, it is safe to exclude the asm dependencies when pulling in ES dependencies in elasticsearch-base. i.e.,

             <dependency>
                  <groupId>org.elasticsearch</groupId>
                  <artifactId>elasticsearch</artifactId>
                  <exclusions>
                      <exclusion>
                          <artifactId>asm</artifactId>
                          <groupId>org.ow2.asm</groupId>
                      </exclusion>
                  </exclusions>
              </dependency>
          

          Adebski have you tried that? Btw, thanks a lot for digging into this

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited Adebski Aljoscha Krettek thanks for the explanation I assumed that the problem was local only because the last time I tried it with cluster execution for the release testing, it worked without issues. I also tried it in local mode just now, however cannot reproduce. I wonder if it may be something specific to the IDE setup, or only surfaces when some other dependency is included. I don't really like the idea of removing the `elasticsearch-base` module and duplicating the base classes. Apart from the duplication which isn't nice, if we're solving it this way, then the problem would still exist for the elasticsearch1 module. As for another possible resolution, I was checking out this thread: https://github.com/elastic/elasticsearch/issues/7959 , and wondering if we can simply exclude the asm dependency from ES. From the comments in that thread, it seems like if we're just using ES client, it is safe to exclude the asm dependencies when pulling in ES dependencies in elasticsearch-base . i.e., <dependency> <groupId>org.elasticsearch</groupId> <artifactId>elasticsearch</artifactId> <exclusions> <exclusion> <artifactId>asm</artifactId> <groupId>org.ow2.asm</groupId> </exclusion> </exclusions> </dependency> Adebski have you tried that? Btw, thanks a lot for digging into this
          Hide
          Adebski Adebski added a comment -

          I'm the person that described the details of the issue on the mailing list.

          Tzu-Li (Gordon) Tai The situation is exactly as you've described: the version of ES configured in elasticsearch-base module is 1.7.1, maven shade plugin detects that this module has ASM as dependency and the shades it under org.apache.flink.shaded package. I've only tested it in local environment because I'm evaluating Flink so it is easier for me to execute my programs directly from IDEA instead of deploying to local cluster. It may work on clustered environment because (correct me if I am wrong) when job is executed in Flink it has a separate classloader in runtime so shaded ASM in version 5.0.4 is available to Flink internal classes and does not clash with 4.X version from ES connector.

          I could work on this issue but we would have to decide the proper course of action. Either remove base module and duplicate the classes or somehow configure build process to not shade ASM in elasticsearch-base module (I've tried to do that but I'm not that proficient in Maven).

          Show
          Adebski Adebski added a comment - I'm the person that described the details of the issue on the mailing list. Tzu-Li (Gordon) Tai The situation is exactly as you've described: the version of ES configured in elasticsearch-base module is 1.7.1, maven shade plugin detects that this module has ASM as dependency and the shades it under org.apache.flink.shaded package. I've only tested it in local environment because I'm evaluating Flink so it is easier for me to execute my programs directly from IDEA instead of deploying to local cluster. It may work on clustered environment because (correct me if I am wrong) when job is executed in Flink it has a separate classloader in runtime so shaded ASM in version 5.0.4 is available to Flink internal classes and does not clash with 4.X version from ES connector. I could work on this issue but we would have to decide the proper course of action. Either remove base module and duplicate the classes or somehow configure build process to not shade ASM in elasticsearch-base module (I've tried to do that but I'm not that proficient in Maven).
          Hide
          aljoscha Aljoscha Krettek added a comment -

          I don't know whether this only occurs in local mode. The reporters mention local mode but they don't say that they didn't encounter it in cluster mode. Maybe they didn't try that because it didn't work in local mode already.

          I think the problem is that flink-connector-elasticsearch-base depends on ASM 4.1, via Elasticsearch 1.7. This is shaded into the flink-connector-elasticsearch-base jar. This then clashes with the shaded ASM 5 dependency that we have in Flink.

          Show
          aljoscha Aljoscha Krettek added a comment - I don't know whether this only occurs in local mode. The reporters mention local mode but they don't say that they didn't encounter it in cluster mode. Maybe they didn't try that because it didn't work in local mode already. I think the problem is that flink-connector-elasticsearch-base depends on ASM 4.1, via Elasticsearch 1.7. This is shaded into the flink-connector-elasticsearch-base jar. This then clashes with the shaded ASM 5 dependency that we have in Flink.
          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited

          I'm not sure I understand what's happening underneath here.
          Is it because elasticsearch5 and elasticsearch2 depends on elasticsearch-base which then pulls in the shaded asm 4.1, which conflicts with Flink's asm dependency?
          If that's the case, wouldn't elasticsearch-base and elasticsearch1 then always bump into this conflict even if we have two completely separate packages?

          Also, I'm curious why this happens only in local mode and not cluster execution.

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited I'm not sure I understand what's happening underneath here. Is it because elasticsearch5 and elasticsearch2 depends on elasticsearch-base which then pulls in the shaded asm 4.1, which conflicts with Flink's asm dependency? If that's the case, wouldn't elasticsearch-base and elasticsearch1 then always bump into this conflict even if we have two completely separate packages? Also, I'm curious why this happens only in local mode and not cluster execution.

            People

            • Assignee:
              Adebski Adebski
              Reporter:
              aljoscha Aljoscha Krettek
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development