Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1605#discussion_r80004187
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -330,11 +328,9 @@ public void ack(Object messageId) {
public void fail(Object messageId) {
final KafkaSpoutMessageId msgId = (KafkaSpoutMessageId) messageId;
emitted.remove(msgId);
- if (msgId.numFails() < maxRetries) {
- msgId.incrementNumFails();
- retryService.schedule(msgId);
- } else { // limit to max number of retries
- LOG.debug("Reached maximum number of retries. Message [{}]
being marked as acked.", msgId);
+ msgId.incrementNumFails();
+ if (!retryService.schedule(msgId)) {
+ LOG.debug("Retry service indicated message should not be
retried. Message [{}] being marked as acked.", msgId);
--- End diff --
@srdo I am not sure I completely follow your reasoning. The most meaningful
piece of information that we want to show to the user here is that if the max
number of retries has been reached, no more retries happen. If the user reads a
message saying that the retry service marked it not to be retried, he still
does not know the cause.
Looking at the code, the `boolean schedule(msgId)` only returns false only
if the max number of retries has been reached.
Although the retry service log messages hints ant the max retries cap being
reached, the user will have to have both logs enabled to have both spouts. I
favor the message as it was before, as it is straight to the point and provides
good insight on what is happening.
---
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.
---
