You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by kailashhd <gi...@git.apache.org> on 2018/03/09 02:27:38 UTC

[GitHub] flink pull request #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS S...

GitHub user kailashhd opened a pull request:

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

    [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for flink kinesis connector

    ## What is the purpose of the change
    
    Updating the AWS Java SDK in order to be able to use new features like ListShards as well as bug fixes as mentioned in [FLINK-8554](https://issues.apache.org/jira/browse/FLINK-8554)
    
    ## Brief change log
      - Update the AWS Java SDK from 1.1.171 to 1.11.272. 
    
    ## Verifying this change
    
    This change is already covered by existing tests, such as FlinkKinesisConsumerMigrationTest.java. Also checked KCL and  KPL for it's dependency version on AWS SDK. KCL 1.8.1 depends on 1.11.171 and KPL  
    depends on 0.12.6 depends on 1.11.128 and it is backward compatible. Also since we shade the dependencies in kinesis connector, we will not have dependencies conflicts with AWS EMR which depends on 1.11.267. Also ran the mvn clean verify.
    
    Tested manually creating a savepoint in 1.3.2 and restarting it in the latest snapshot. 
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (**yes** / no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
      - The serializers: (yes / **no** / don't know)
      - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
      - The S3 file system connector: (yes / **no** / don't know) - AWS Java SDK changed only for Kinesis
    ## Documentation
    
      - Does this pull request introduce a new feature? (yes / **no**)
      - If yes, how is the feature documented? (not **applicable** / docs / JavaDocs / not documented)


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

    $ git pull https://github.com/kailashhd/flink UpdateAWSSDK

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

    https://github.com/apache/flink/pull/5663.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 #5663
    
----
commit 47d0e5523ca908bc5b11c21902ea1a5385914644
Author: Kailash HD <kd...@...>
Date:   2018-03-08T18:32:23Z

    [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for flink kinesis connector

----


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by kailashhd <gi...@git.apache.org>.
Github user kailashhd commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Currently in flink connector we are depending only on aws-sdk-kinesis and not on aws-java-sdk-bundle and also don't depend on kinesisvideo. So by default the dependency on kinesisvideo is not included in the connector which means we don't have to exclude any dependencies. I also verified that there is no unwanted netty dependencies by running mvn dependency:tree. The only instance of netty is this: `[INFO] |  +- org.apache.flink:flink-shaded-netty:jar:4.0.27.Final-2.0:provided` in accordance to the value in flink-parent pom.


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Travis CI pending after rebasing this, will push as soon at it gives us a green light...


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Looks reasonable.
    
    Would be great if another Kinesis user could second this change, just to be safe.
    @tweise do you see any issues with upgrading AWS SDK here?


---

[GitHub] flink pull request #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS S...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    you don't need to shade it, just exclude it explicilty in your pom .it came in with kinesis-video, and only stops that feature from working if excluded [AWS 1488](https://github.com/aws/aws-sdk-java/issues/1488)


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by kailashhd <gi...@git.apache.org>.
Github user kailashhd commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Sorry for the delay in this. Confirming that I tested this using kinesalite and kinesis for both the consumer and producer functionality. I had some trouble when connecting this to kinesalite due to the following issue for consumers [FLINK-8936]. I am planning on fixing this in the next iteration and did not want for cleaner modular PRs. 


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by tweise <gi...@git.apache.org>.
Github user tweise commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    @StephanEwen we are looking to utilize this patch to override discovery using ListShards (initially in our connector extension). 
    
    I have tested the SDK version change on top of our Flink 1.4 + patches branch with my kinesis quick check app and it works as expected.
    
    I believe @kailashhd had already addressed all other concerns?
    
    



---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    I think the above means we need to check and either manually shade netty or, preferably, exclude the netty dependency (if it is not actually needed for Kinesis)


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Forwarding comment from JIRA from @steveloughran
    
    > If you are pulling in the shaded SDK, note that it's been declaring its netty dependencies unshaded: HADOOP-15264


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by bowenli86 <gi...@git.apache.org>.
Github user bowenli86 commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Is your testing Flink job both reading from and writing to Kinesis, aka both KCL and KPL are tested?
    
    If so, +1


---

[GitHub] flink issue #5663: [FLINK-8888] [Kinesis Connectors] Update the AWS SDK for ...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5663
  
    Thanks for checking this out. Merging to 1.5 and 1.6 then...


---