You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tzulitai <gi...@git.apache.org> on 2017/03/15 16:10:00 UTC
[GitHub] flink pull request #3549: [FLINK-5701] [kafka] FlinkKafkaProducer should che...
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/3549
[FLINK-5701] [kafka] FlinkKafkaProducer should check asyncException on checkpoints
This is a backport of FLINK-5701 for `release-1.1`.
The added tests are identical to #3278, with only minor changes to fit with the state of the producer in the `release-1.1` branch.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tzulitai/flink FLINK-5701-1.1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3549.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 #3549
----
commit e4d9a14dc45200c00806377cdd34f551870213ef
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date: 2017-03-15T16:05:51Z
[FLINK-5701] [kafka] FlinkKafkaProducer should check asyncException on checkpoints
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProducer sh...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3549
Thanks for the review!
Travis is green (local branch), with only test timeouts: https://travis-ci.org/tzulitai/flink
Merging this ..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106221523
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -240,6 +240,7 @@ public void open(Configuration configuration) {
}
if (flushOnCheckpoint && !((StreamingRuntimeContext)this.getRuntimeContext()).isCheckpointingEnabled()) {
+ System.out.println(((StreamingRuntimeContext)this.getRuntimeContext()).isCheckpointingEnabled());
--- End diff --
Yes indeed, will remove.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106221016
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -240,6 +240,7 @@ public void open(Configuration configuration) {
}
if (flushOnCheckpoint && !((StreamingRuntimeContext)this.getRuntimeContext()).isCheckpointingEnabled()) {
+ System.out.println(((StreamingRuntimeContext)this.getRuntimeContext()).isCheckpointingEnabled());
--- End diff --
This looks like a debug message
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106221305
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -374,4 +380,11 @@ public static Properties getPropertiesFromBrokerList(String brokerList) {
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerList);
return props;
}
+
+ // this is exposed for testing purposes
+ protected long numPendingRecords() {
--- End diff --
There is the `@VisibleForTesting` annotation for that :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai closed the pull request at:
https://github.com/apache/flink/pull/3549
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProducer sh...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/3549
Backport is +1 to merge, once the sysout has been removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106222093
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -374,4 +380,11 @@ public static Properties getPropertiesFromBrokerList(String brokerList) {
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerList);
return props;
}
+
+ // this is exposed for testing purposes
+ protected long numPendingRecords() {
--- End diff --
No, you are right :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106221979
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -374,4 +380,11 @@ public static Properties getPropertiesFromBrokerList(String brokerList) {
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerList);
return props;
}
+
+ // this is exposed for testing purposes
+ protected long numPendingRecords() {
--- End diff --
I don't think its available in `release-1.1`, is it? I might be mistakened, though ..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106222066
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -374,4 +380,11 @@ public static Properties getPropertiesFromBrokerList(String brokerList) {
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerList);
return props;
}
+
+ // this is exposed for testing purposes
+ protected long numPendingRecords() {
--- End diff --
yup :-D
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3549: [backport-1.1] [FLINK-5701] [kafka] FlinkKafkaProd...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3549#discussion_r106221830
--- Diff: flink-streaming-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerBase.java ---
@@ -374,4 +380,11 @@ public static Properties getPropertiesFromBrokerList(String brokerList) {
props.setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerList);
return props;
}
+
+ // this is exposed for testing purposes
+ protected long numPendingRecords() {
--- End diff --
Damn .. This is Flink 1.1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---