You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by skyahead <gi...@git.apache.org> on 2016/04/01 06:36:04 UTC

[GitHub] flink pull request: [FLINK-3541] [Kafka Connector] Clean up workar...

GitHub user skyahead opened a pull request:

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

    [FLINK-3541] [Kafka Connector] Clean up workaround in FlinkKafkaConsu…

    …mer09

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

    $ git pull https://github.com/skyahead/flink FLINK-3541

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

    https://github.com/apache/flink/pull/1846.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 #1846
    
----
commit 49c291a5bd06f1468bcee40f03a7bbea3bb1be29
Author: Tianji Li <sk...@gmail.com>
Date:   2016-04-01T04:35:39Z

    [FLINK-3541] [Kafka Connector] Clean up workaround in FlinkKafkaConsumer09

----


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1846#discussion_r58177554
  
    --- Diff: flink-streaming-connectors/flink-connector-kafka-0.9/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumer09.java ---
    @@ -36,6 +36,7 @@
     import org.apache.kafka.common.PartitionInfo;
     import org.apache.kafka.common.TopicPartition;
     import org.apache.kafka.common.errors.WakeupException;
    ++import org.apache.kafka.common.KafkaException;
    --- End diff --
    
    Extra `+`. See also CI logs...


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/1846#issuecomment-205270862
  
    Apparently https://issues.apache.org/jira/browse/KAFKA-2880 has been fixed with Kafka 0.9.0.1. Still, we should make sure that the multiple (I'd say at least 10) test runs are passing without failure.
    
    I'll look into this once its building.


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

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

    https://github.com/apache/flink/pull/1846#issuecomment-205272651
  
    @skyahead Will you be fixing the compile issue of this pull request?


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

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

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


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/1846#issuecomment-205811304
  
    Yes, we can drop the retry loop.


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

Posted by skyahead <gi...@git.apache.org>.
Github user skyahead commented on the pull request:

    https://github.com/apache/flink/pull/1846#issuecomment-205297233
  
    @StephanEwen Sorry for late replying. Did not get much time during the weekend on keyboard :-) .
    
    I was trying to understand the test cases related, to address your feedback regarding 'logic being changed'. 
    
    Will work on this from tonight!


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

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

    https://github.com/apache/flink/pull/1846#issuecomment-204310567
  
    This seems to change the behavior. Are you sure that the current workaround (catching the `NullPointerException`) is not needed any more?


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

Posted by skyahead <gi...@git.apache.org>.
Github user skyahead commented on the pull request:

    https://github.com/apache/flink/pull/1846#issuecomment-205604300
  
    @StephanEwen I changed Kafka version from 0.9.0.1 to 0.9.0.0 and verified that NullPointerException does get caught, and the code retries connecting for 10 times. 
    
    Using 0.9.0.1 however, the NullPointerException does not happen anymore, whereas a TimeoutException is thrown as expected and got caught expected too.
    
    All test cases in Kafka08ITCase (19 cases) and Kafka09ITCase (15 cases) pass in my local environment.


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

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

    https://github.com/apache/flink/pull/1846#issuecomment-208849988
  
    Okay, will merge this on top of the recent changes to the Kafka consumer...


---
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: [FLINK-3541] [Kafka Connector] Clean up workar...

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

    https://github.com/apache/flink/pull/1846#issuecomment-205744035
  
    @rmetzger Given that the Kafka bug is fixed, can we drop the re-try loop, or was that intended to also cover other cases (like brokers having inconsistent metadata) ?


---
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.
---