You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by srdo <gi...@git.apache.org> on 2017/09/02 22:09:13 UTC

[GitHub] storm pull request #2307: STORM-2546: Fix storm-kafka-client spout getting s...

GitHub user srdo opened a pull request:

    https://github.com/apache/storm/pull/2307

    STORM-2546: Fix storm-kafka-client spout getting stuck when retriable offsets were deleted from the Kafka log due to topic compaction

    This requires the changes in https://github.com/apache/storm/pull/2156 for correctness, please ignore the first commit.
    
    Here's the idea behind these changes https://issues.apache.org/jira/browse/STORM-2546?focusedCommentId=16151172&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16151172
    
    KafkaSpoutCommitTest was renamed, so the first test in KafkaSpoutLogCompactionSupportTest was already there and can be skipped.

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

    $ git pull https://github.com/srdo/storm STORM-2546

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

    https://github.com/apache/storm/pull/2307.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 #2307
    
----
commit 5d13d3a1142d808107e630479514a922904f2187
Author: Stig Rohde Døssing <st...@gmail.com>
Date:   2017-06-10T17:38:22Z

    STORM-2549: Fix broken enforcement mechanism for maxUncommittedOffsets in storm-kafka-client spout

commit 19a6c4e8824bbf1c98d36e4073e68631ece9047b
Author: Stig Rohde Døssing <sr...@apache.org>
Date:   2017-09-02T21:50:33Z

    STORM-2546: Fix storm-kafka-client spout getting stuck when retriable tuples were deleted from the Kafka log due to topic compaction

----


---
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] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @srdo 
    I found same suggestion as my 2 cents is already discussed from JIRA issue. Is it already addressed?


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @srdo 
    Sure. Makes sense. Let's add a warning message.


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @askprasanna I've made the spout default to an auto offset reset policy of "earliest" when the user requests the at-least-once processing guarantee. I think this addresses your request here https://issues.apache.org/jira/browse/STORM-2546?focusedCommentId=16152824&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16152824.


---

[GitHub] storm pull request #2307: STORM-2546: Fix storm-kafka-client spout getting s...

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

    https://github.com/apache/storm/pull/2307


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @srdo Awesome. Thanks for following up.


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @HeartSaVioR Merged to master. Yes, I'll put up a 1.x version tonight.


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @HeartSaVioR Added the warning. Will squash and merge if the warning looks good.


---

[GitHub] storm issue #2307: STORM-2546: Fix storm-kafka-client spout getting stuck wh...

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

    https://github.com/apache/storm/pull/2307
  
    @HeartSaVioR Yes, you are right. We set the auto offset reset policy to `earliest` by default when `AT_LEAST_ONCE` is picked if the user hasn't explicitly set anything else. 
    
    The misconfiguration I'm most worried about is where users set `AT_LEAST_ONCE`, and then forget to set the auto offset reset policy which defaults to `latest`, because it's an easy mistake to make if you're not already very familiar with Kafka's options. The other weird configurations (`AT_LEAST_ONCE`+`latest`, `AT_MOST_ONCE`+`earliest`) have to be explicitly chosen by the user.
    
    I can't think of a reason why users would want to use those configurations, but I thought it might be better not to prevent the user from using those settings if they really want to, because there might be a use case I'm not seeing.
    
    I'm happy to add in checks and error messages (or maybe even throwing exceptions) when using those configurations, if you think it makes sense?


---