Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1807
@hmcl Yes, the case I'm thinking about is where the topology has acking
enabled, but the spout is set to auto-commit mode. It would probably be fine to
do best-effort retrying when using that setup, but it seems like kind of an odd
case to support. The retry code needs to be changed to support that case as
well, since doSeekRetriableTopicPartitions doesn't work at all when auto-commit
is enabled, because it depends on acked, so I'm not really sure it's worth it.
Regarding the patch and my previous comment, I still think moving this
check into fail() and just disabling retries if auto commit is enabled is a
better option (i.e. if auto-commit is on, the only thing fail() needs to do is
remove the message from emitted and nothing else). It has the same effect as
this patch, but we can ensure that it only has effect if auto-commit is
enabled. The patch as is could end up hiding bugs, since
doSeekRetriableTopicPartitions now quietly skips topicpartitions where acked is
null. It's also easier to tell from reading the code why failing tuples would
be disabled when auto-commit is on, than it is to tell why there's a null check
in doSeekRetriableTopicPartitions.
---
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.
---
