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 [email protected] or file a JIRA ticket
with INFRA.
---
