consumer: LogFailedMessage behavior

[Joevychow]

When HandleMessage handlers a message and returns an error, the message will requeue by default. But when HandleMessage return errrors five times(default MaxAttempts), why requeue again and then triggler LogFailedMessage instead of triggler immediately?

[ploxiln]

When a message is "failed", the way further attempts are prevented is with message.Finish(). The message attempts policy is checked and enforced by the consumers, not by nsqd, and nsqd can't really tell the difference between final failures and final successes.

        if r.shouldFailMessage(message, handler) {
            message.Finish()
            continue
        }

(The optional LogMessageFailed method is called at the same time, from shouldFailMessage().)

As you noticed, this is checked just before calling the actual message handling function, instead of just after. So after the last time the message is actually processed, it is requeued once more, then redelivered once more, and then failed/finished. The reason why it is done this way is:

• If the check is after the message handling function, and if the message caused a hard crash or indefinite hang of the consumer process, it would never be finished. It would time-out and be re-delivered, indefinitely, because it would never get to the check which happens after the handler function.
• A common case is when the consumer just takes too long, and nsqd times-out waiting for the consumer to finish, then nsqd will not accept a finish() at the end of processing, after the timeout. It may have already sent the message elsewhere, but anyway, never allows a consumer to affect a message that is not in-flight to that consumer. The next consumer which gets the message will be the one to notice the attempts count is exceeded (due to timeouts in this case) before it starts trying to process the message.
• I think we didn't want to put the check in two places (both before and after) and thought that only before was good enough. If most messages are processed successfully, there isn't much overhead from extra requeues. But it is plausible that attempts could be checked afterwards too, if a message is explicitly requeued by the handler function. This could be discussed in a pull-request adding this optimization.