Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-60354121
Hi @wurstmeister. I like the way yours encapsulates the collections into
the separate <tt>FailedMessageHandler</tt> class. That's definitely cleaner.
I have two comments:
<ol>
<li>Exponential backoff for retries seems to be pretty universal, and you
can achieve both the current behavior (instant retries) and constant time
retries by choosing appropriate parameters for the exponential backoff. So I'm
not sure if defining an interface and creating multiple implementations is
worth it for this problem.</li>
<li>Although I can imagine that you might want to set a <tt>maxRetries</tt>
value, it definitely should be possible to tell the spout to retry
indefinitely. The reason is that when you stop retrying, you're generally
going to want to log the error that is happening and probably respond in some
application-specific way. So the act of terminating the retries is really best
left to the application-specific bolt in the topology where the error is
occurring. In my setup, each of my bolts keeps track of retries that it has
caused, and after a certain number of tries it saves some information to an
error kafka queue and acks the message.</li>
</ol>
---
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.
---
