grssam commented on PR #23398:
URL: https://github.com/apache/pulsar/pull/23398#issuecomment-2453555618
> Umm.. I am still not clear. Can't we send THROTTLE_ERROR_CODE and handle
it instead of introducing new command?
Could you please elaborate a bit more? Not 100% sure what you mean here..
Are you saying that we simply treat all timeouts as throttling issue and return
THROTTLE_ERROR_CODE instead of TIMEOUT without the need of server sending any
info out to the client (via the new command?)
> Correct. In that case, check client-version at broker side and don't
trigger this workflow if client doesn't support the version. So, we don't need
ACK response back from client. For information: check `ServerCnx.java -> int
clientProtocolVersion = connect.getProtocolVersion();`
Actually, i checked around this on slack. seems like feature flags are the
new approach of handling this and protocol version only bumps up in case of
major changes in the protocol.
In fact, I am actually using this feature flag approach to not even bother
sending this command to clients who have not implemented handling of this new
command.
The concern still remains about bad actors - intentional or unintentional.
Intentional - a client says it supports the handling of this throttle
command but still misbehaves due to code bug, or any other reason.
Unintentional - TCP is a 2 way stream (or internally, it opens 2 channels)..
there can be a choking of outbound messages from broker, thus the throttle
command gets held up and clients continue to bombard the broker.
Both of the above situations lead to noisy neighbor issue on the broker side
and the client side alike. It essentially leads to no rate limiting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
