feat(server): support the client-output-buffer-limit configuration#3560
feat(server): support the client-output-buffer-limit configuration#3560git-hulk wants to merge 1 commit into
Conversation
Add the client-output-buffer-limit configuration to disconnect clients
that are not reading data from the server fast enough, following the
same semantics as Redis: a client is disconnected immediately once the
hard limit is reached, or when it stays over the soft limit for more
than the configured seconds continuously.
Unlike Redis, all the classes are configured in a single line, and only
the specified classes are changed. The limits are disabled by default
for all classes of clients, e.g. to protect against slow Pub/Sub
subscribers with the same limits as Redis:
client-output-buffer-limit pubsub 32m 8m 60
The limit takes effect on the normal and pubsub classes since the
replication stream is written to the socket directly instead of going
through the connection output buffer. The slave class is accepted for
compatibility but takes no effect: slow replicas are still handled by
max-replication-lag and replication-send-timeout-ms.
The check runs every time data is appended to the connection output
buffer. A client over the limit is closed asynchronously by manually
triggering the deferred write callback, since the write callback of a
stuck client may never fire on its own; the callback then runs in the
owner worker's event loop where freeing the connection is safe.
Also expose the client_output_buffer_limit_disconnections counter in
INFO stats, and classify clients subscribed to only shard channels as
pubsub clients so that they are covered by the pubsub class as well.
Assistant-By Claude Fable 5
git-hulk
force-pushed
the
add-client-output-buffer-limit
branch
from
737c793 to
bea7369
Compare
|
jihuayu
left a comment
jihuayu
left a comment
There was a problem hiding this comment.
The async close is not guaranteed to invoke Connection::OnWrite, because blocking commands replace the bufferevent callbacks.
For example, with:
client-output-buffer-limit pubsub 1m 0 0
a RESP3 client can run:
HELLO 3
SUBSCRIBE ch
BLPOP never-exists 0
and another client can publish an 8 MiB message to ch.
I reproduced this on this PR. BlockingCommander::StartBlocking() had replaced the write callback, so bufferevent_trigger(..., EV_WRITE, ...) invoked BlockingCommander::OnWrite() instead of Connection::OnWrite(). Since the list was still empty, it disabled EV_WRITE and returned without checking kCloseAsync.
After 12 seconds the client was still present in CLIENT LIST with:
cmd=blpop obuf=8388658
and client_output_buffer_limit_disconnections was already 1. The connection was only removed after the test client closed its socket. A non-blocked subscriber was closed normally in the control test.
| } else { | ||
| return {Status::NotOK, fmt::format("unknown client kind '{}'", args[i])}; | ||
| } | ||
| auto hard = GET_OR_RET(ParseSizeAndUnit(args[i + 1]).Prefixed("invalid hard limit")); |
There was a problem hiding this comment.
It looks a little different from Redis: https://github.com/redis/redis/blob/e5e1eaa97ad3020305b84442197011abc8ce40bc/redis.conf#L8-L18
|
|
||
| if (soft_limit_bytes != 0) { | ||
| if (used_bytes >= soft_limit_bytes) { | ||
| auto soft_limit_seconds = static_cast<int64_t>(limit.soft_limit_seconds.load(std::memory_order_relaxed)); |
There was a problem hiding this comment.
Can we use the same type for limit.soft_limit_seconds and soft_limit_seconds to avoid overflow?
2 participants
Add the client-output-buffer-limit configuration to disconnect clients that are not reading data from the server fast enough, following the same semantics as Redis: a client is disconnected immediately once the hard limit is reached, or when it stays over the soft limit for more than the configured seconds continuously.
Unlike Redis, all the classes are configured in a single line, and only the specified classes are changed. The limits are disabled by default for all classes of clients, e.g. to protect against slow Pub/Sub subscribers with the same limits as Redis:
The limit takes effect on the normal and pubsub classes since the replication stream is written to the socket directly instead of going through the connection output buffer. The slave class is accepted for compatibility but takes no effect: slow replicas are still handled by max-replication-lag and replication-send-timeout-ms.
The check runs every time data is appended to the connection output buffer. A client over the limit is closed asynchronously by manually triggering the deferred write callback, since the write callback of a stuck client may never fire on its own; the callback then runs in the owner worker's event loop where freeing the connection is safe.
Also expose the client_output_buffer_limit_disconnections counter in INFO stats, and classify clients subscribed to only shard channels as pubsub clients so that they are covered by the pubsub class as well.
Part of #2284
Assistant-By Claude Fable 5