Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1679#discussion_r79794331 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -479,16 +482,17 @@ public OffsetAndMetadata findNextCommitOffset() { KafkaSpoutMessageId nextCommitMsg = null; // this is a convenience variable to make it faster to create OffsetAndMetadata for (KafkaSpoutMessageId currAckedMsg : ackedMsgs) { // complexity is that of a linear scan on a TreeMap - if ((currOffset = currAckedMsg.offset()) == initialFetchOffset || currOffset == nextCommitOffset + 1) { // found the next offset to commit + if ((currOffset = currAckedMsg.offset()) == nextCommitOffset + 1) { // found the next offset to commit --- End diff -- @srdo from what I recall, in scenarios where topic had no messages at all, this was necessary. I tuned some of these changes based on several integration tests I ran under several scenarios. As I mentioned in a prior comment, I have been in touch with people that are running this in production without any issues. You are proposing quite a few algorithmic changes. Are these changes caused by bugs ? Do you have test scenarios that guarantee that the proposed changes are not adding new bugs ? Or are these changes proposed by code analysis that leads you to believe that they are not necessary ?
--- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---