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

[FLINK-7366][kinesis connector] Upgrade kinesis producer library in flink-connector-kinesis #4522

Closed
wants to merge 3 commits into from

Conversation

bowenli86
Copy link
Member

@bowenli86 bowenli86 commented Aug 10, 2017

What is the purpose of the change

We need to upgrade KPL and KCL to pick up the enhanced performance and stability for Flink to work better with Kinesis. Upgrading KPL is specially necessary, because the KPL version Flink uses is old, and doesn't have good retry and error handling logic.

Upgrade KPL:

flink-connector-kinesis currently uses kinesis-producer-library 0.10.2, which is released in Nov 2015 by AWS. It's old. It's the fourth release, and thus problematic. It doesn't even have good retry logic, therefore Flink fails really frequently (about every 10 mins as we observed) when Flink writes too fast to Kinesis and receives RateLimitExceededException,

Quotes from awslabs/amazon-kinesis-producer#56, "With the newer version of the KPL it uses the AWS C++ SDK which should offer additional retries." on Oct 2016. 0.12.5, the version we are upgrading to, is released in May 2017 and should have the enhanced retry logic.

Upgrade KCL:

Upgrade KCL from 1.6.2 to 1.8.1

Upgrade AWS SDK:

from 1.10.71 to 1.11.171

Verifying this change

This change is already covered by existing tests

Does this pull request potentially affect one of the following parts:

There should be any impact outside flink-connector-kinesis, because 1) KPL and KCL is only used in flink-connector-kinesis, and 2) AWS SDK in flink-connector-kinesis is shaded

Documentation

  • Does this pull request introduce a new feature? (no)

@bowenli86 bowenli86 changed the title [FLINK-7366][kinesis connector] Upgrade kinesis-producer-library in flink-connector-kinesis from 0.10.2 to 0.12.5 [FLINK-7366][kinesis connector] Upgrade kinesis producer library, kinesis client library, and AWS SDK in flink-connector-kinesis Aug 10, 2017
@tzulitai
Copy link
Contributor

testRestoreFromFlink11 is failing for the Kinesis consumer, since in Flink 1.1 we were directly serializing Kinesis classes in savepoints. Upgrading the Kinesis dependency would mean that the 1.1 savepoints would not be able to be restored.

I think, however, that there recently was a poll on the mailing list to remove 1.1 savepoint compatibility support starting from 1.4. That would entail we can completely remove the restore 1.1 tests in master.

@aljoscha
Copy link
Contributor

@tzulitai Are we not using Java Serialisation starting from 1.2.x?

@tzulitai
Copy link
Contributor

tzulitai commented Aug 11, 2017

Hmm, I think Java serialization of Kinesis classes was removed from the Kinesis connector starting from 1.3.
But thinking of it now, I recall that there was some migration path from that. Let me double check ..

@tzulitai
Copy link
Contributor

Ok, so it seems like Java serialization of the Kinesis classes was removed only starting from 1.3.x.
That means unless we do some classloading trick, we can't really upgrade the Kinesis consumer dependencies until 1.2 savepoint compatibility is no longer supported.

@bowenli86
Copy link
Member Author

oh.....this is not good....Do you mean that we cannot upgrade KPL/KCL until Flink 1.5?

@bowenli86
Copy link
Member Author

@tzulitai @aljoscha there's another major bug in KPL that we need to fix for flink-connector-kinesis awslabs/amazon-kinesis-producer#10 We've been seeing that bug crashing our first Flink cluster all the time. It's blocking us from making our first Flink cluster production-ready.

@tzulitai
Copy link
Contributor

@bowenli86 its fine to upgrade KPL. It's consumer related libraries version that currently cannot be touched.

Consumer related versions can currently only be upgraded until 1) Flink 1.2 is no longer supported, or 2) we have the state migration feature available.

@bowenli86
Copy link
Member Author

@tzulitai OK, I only upgraded KPL

@tzulitai
Copy link
Contributor

tzulitai commented Aug 14, 2017

Quick check before merging this, @bowenli86:
Was this already tested in actual AWS environments? I just want to make sure there's no dependency conflicts or similar culprits before merging.

(We usually would want to do an "real" test run when upgrading dependency versions).

@bowenli86
Copy link
Member Author

yeah, we ran our job with this change on Flink 1.3.0 on EMR and didn't have issues.

@bowenli86 bowenli86 changed the title [FLINK-7366][kinesis connector] Upgrade kinesis producer library, kinesis client library, and AWS SDK in flink-connector-kinesis [FLINK-7366][kinesis connector] Upgrade kinesis producer library in flink-connector-kinesis Aug 15, 2017
@tzulitai
Copy link
Contributor

@bowenli86 thanks for confirming! Merging this also then ..

tzulitai pushed a commit to tzulitai/flink that referenced this pull request Aug 16, 2017
tzulitai pushed a commit to tzulitai/flink that referenced this pull request Aug 18, 2017
@bowenli86
Copy link
Member Author

@tzulitai any more feedbacks? We have a ticket on my company for this task, and I'd like to mark it as finished if possible :)

@bowenli86 bowenli86 closed this Aug 22, 2017
@bowenli86 bowenli86 reopened this Aug 22, 2017
@bowenli86
Copy link
Member Author

bowenli86 commented Aug 29, 2017

can anyone from PMC take a look at this PR please?

rmetzger pushed a commit to rmetzger/flink that referenced this pull request Aug 29, 2017
@rmetzger
Copy link
Contributor

I've pushed the PR to CI again. If it passes, I'll merge it.

@bowenli86
Copy link
Member Author

Great! I can start FLINK-7508 then

@aljoscha
Copy link
Contributor

@tzulitai Is currently on vacation but before he left he added a Backwards compatibility test for the Kinesis consumer. We also dropped compatibility with the Flink 1.2 Kinesis consumer so I think we can also update the consumer library. (I realised this is not completely related to this PR but I just noticed it now, sorry for that.)

@asfgit asfgit closed this in b915757 Aug 30, 2017
@bowenli86
Copy link
Member Author

@aljoscha No worries. Dropping compatibility with Flink 1.2 Kinesis consumer is great news! I reopened (Flink-7422)[https://issues.apache.org/jira/browse/FLINK-7422] to upgrade KCL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants