-
Notifications
You must be signed in to change notification settings - Fork 716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementation of write throttling #1672
base: unstable
Are you sure you want to change the base?
Implementation of write throttling #1672
Conversation
05a7519
to
37101df
Compare
We will make the cut-off for 8.1 release candidates in a few days, so this will most likely have to wait until the next release. Other than review, we'll need to make some decision about the feature and about throttling in general.
Regarding this problem, we already have dual-channel replication feature introduced in 8.0 which offloads the buffering to the replica side. I think it can help to some extent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. If TSC approves it, we would also need to add CLIENT opt in flag to enable this feature for client, that can handle the feature..
@@ -2059,6 +2060,7 @@ void createSharedObjects(void) { | |||
createObject(OBJ_STRING, sdsnew("-EXECABORT Transaction discarded because of previous errors.\r\n")); | |||
shared.noreplicaserr = createObject(OBJ_STRING, sdsnew("-NOREPLICAS Not enough good replicas to write.\r\n")); | |||
shared.busykeyerr = createObject(OBJ_STRING, sdsnew("-BUSYKEY Target key name already exists.\r\n")); | |||
shared.throttlederr = createObject(OBJ_STRING, sdsnew("-THROTTLED\r\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending a generic throttled error seems risky to let each client build their own retry logic. Either we need to send out an estimated replication duration as retry-after time or maintain a backoff period for each client and reset it once the replication backlog gets cleared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or block the client with a configurable timeout, then either resume writing if COB pressure subsides or return an error.
Signed-off-by: Lucas Schmidt Cavalcante <[email protected]>
Signed-off-by: Lucas Schmidt Cavalcante <[email protected]>
Signed-off-by: Lucas Schmidt Cavalcante <[email protected]>
acd833a
to
de280f6
Compare
I'm not sure this approach is ideal for multi-replica setups. A single problematic replica could render the engine unusable, even when losing one replica might be acceptable. |
Signed-off-by: Lucas Schmidt Cavalcante <[email protected]>
Signed-off-by: lschmidtcavalcante-sc <[email protected]>
This feature (config
write-throttling
) aims to give an alternative to client disconnection when the output buffer overflows. Once enabled, it evaluates if the incoming write command will exceed the output buffer limit of any replica: if it will exceed, then discard the write command and returnTHROTTLED
; otherwise write it to the output buffer.The expected main usage of this feature is during replication: while the replica is busy copying the initial snapshot its output buffer may overflow, causing a restart of the replication process. At the cost of dropping some requests (responding as
THROTTLED
) an endless replication loop can be averted.Issue #1649