nsqd: don't decrement RDY

[mreiferson]

This is something @jehiah and I have discussed for some time, after writing a few client libraries and talking to other client library authors.

In retrospect, we've realized that it isn't actually necessary for nsqd to decrement the RDY count for a given client, forcing client libraries to resend RDY periodically.

It's not the actual decrementing of the RDY count that acts as flow control but rather the fact that nsqd will not send any more than the specified value in-flight coupled with the fact that a consumer needs to respond to each message.

The proposal is to remove this internal implementation detail, which should not impact nsqd, client libraries, or consumers in any meaningful way.

cc @jehiah

[ploxiln]

ignorant bystander's opinion: 👍

[mreiferson]

now that we've stamped stable... RFR 😁

[jehiah]

In favor of this.

As i think about it, i think the situation this breaks existing expectations client wise is that RDY 1 only sends a single message to "test the waters" when in a backoff state and that there isn't a need to send RDY 0 as soon as you get that message to stop further messages.

That makes me feel like we should update clients to explicitly zero out their RDY states at the right points before this lands.

Another case this behaves differently is if you have a client that is stuck on a message. Say you have RDY 1, 10s msg timeouts and a client gets stuck on a message for 5 minutes. In that case when nsqd times out the message it will start pumping more messages to the client (and timing them out every 10s) where previously the client would have stalled until it finished it's work and renewed it's RDY count (unless it preemptively renewed RDY count upon receipt of the message in which case functionality is the same). We could possibly mitigate with server side backoff on client timeouts, but i'm not sure i like how things get handled in this situation.

[mreiferson]

As i think about it, i think the situation this breaks existing expectations client wise is that RDY 1 only sends a single message to "test the waters" when in a backoff state and that there isn't a need to send RDY 0 as soon as you get that message to stop further messages.

It wouldn't continue to send messages unless that first message was responded to, so I don't think zero'ing out RDY is actually necessary here.

Another case this behaves differently is if you have a client that is stuck on a message. Say you have RDY 1, 10s msg timeouts and a client gets stuck on a message for 5 minutes. In that case when nsqd times out the message it will start pumping more messages to the client (and timing them out every 10s) where previously the client would have stalled until it finished it's work and renewed it's RDY count (unless it preemptively renewed RDY count upon receipt of the message in which case functionality is the same). We could possibly mitigate with server side backoff on client timeouts, but i'm not sure i like how things get handled in this situation.

This situation also occurred to me and I agree that it isn't ideal, but practically all of our client libraries already result in this behavior because the sending of RDY is essentially decoupled from responding to messages (it's performed "up front"), save for some edge cases.

[jehiah]

For the first item, my point is that the whole point of client backoff is that you expect no messages and that won't happen any longer. You'll still get them streamed in as fast as you can respond (with no parallelism in the RDY 1 case i described).

[mreiferson]

There's a small window for a single additional message, yes, but the client libraries immediately send a RDY 0 (or full throttle) based on the backoff state after each "test the water" message is responded to.

Which I think is perfectly OK.

[jehiah]

hmm ok i'll re-familiarize myself with that handling.

[mreiferson]

@jehiah hit the button?

[jehiah]

🔥