Add an optional fast checker to (*partitionConsumer).responseFeeder

[wmille]

Add a ticker in (*partitionConsumer).responseFeeder to frequently check for
messages before starting the timeout timer. Using the fast checker significantly
reduces the number of timer function calls when timeouts are infrequent.

[eapache]

Interesting PR, thanks for looking at performance here. Do you have any benchmarks or profiles showing the call to expiryTimer.Reset as a particular bottleneck? In our experience it's usually quite overshadowed by the latency of the network, and the cost of whatever processing the consuming code actually does with the message.

I'm also a bit concerned by the complexity of the implementation here. Have you considered just converting the expiryTimer itself into a ticker? That would remove the need to maintain two channels and switch back and forth, at the cost of decreasing the accuracy of the timeout even further (to between MaxProcessingTime and 2 * MaxProcessingTime), but accuracy is basically irrelevant as long as it still prevents the consumer from getting stuck entirely.

[wmille]

go tool pprof shows about 20% of total program CPU time being spent in timer functions ((*Timer).Reset and (*Timer).Stop) called from (*partitionConsumer).responseFeeder.

We considered converting the expiryTimer into a ticker but thought that the loss of accuracy would be an issue. I could change the implementation to use that logic if the loss of accuracy would be acceptable.

[eapache]

Ya, I think that would be fine as long as we're clear about it in the documentation. The implementation will definitely be a lot easier to follow.

[eapache]

To clarify a little further: the timer is already very inaccurate because of the way it interacts with the buffer size of the messages channel (you could, in theory, get accuracy by setting the buffer size to 0, but I don't know of anybody who does nor can I imagine a use case for it).

Given this, I would be OK with just switching to the fast-checker and not adding a configuration option. Thoughts?

[wmille]

That makes sense. The configuration option was just left in because this change would technically change the timeout behavior (even though it is already very inaccurate as you described). I'll go ahead and make that change.

[eapache]

Thanks!