Limit repl buffer writes per event loop (#256)
We discovered that full sync loops were happening because after the replica loads the RDB file, the repl backlog which the primary sends over can sometimes exceed int32 max, causing an SSL error due to integer overflow: ``` Error writing to client: error:140D010F:SSL routines:SSL_write:bad length ``` After fixing that error by capping the and testing in staging, we still observed errors because the primary attempts to send a massive (multi-GB) payload in one event loop, which on the staging instances takes over one minute causing the primary to fail health checks and restart. To address this we cap the writes sent per event loop to a configurable amount, similar to `NET_MAX_WRITES_PER_EVENT` in the normal case.
This commit is contained in:
1 parent
195828ed4c
commit
cba39986c1
4 files changed
+18
-6
No files matched your search
@@ -3158,6 +3158,7 @@ standardConfig configs[] = {
|
||||
createLongLongConfig("repl-backlog-disk-reserve", NULL, IMMUTABLE_CONFIG, 0, LLONG_MAX, cserver.repl_backlog_disk_size, 0, MEMORY_CONFIG, NULL, NULL),
|
||||
createLongLongConfig("max-snapshot-slip", NULL, MODIFIABLE_CONFIG, 0, 5000, g_pserver->snapshot_slip, 400, 0, NULL, NULL),
|
||||
createLongLongConfig("max-rand-count", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX/2, g_pserver->rand_total_threshold, LONG_MAX/2, 0, NULL, NULL),
|
||||
createLongLongConfig("repl-backlog-max-writes-per-event", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->repl_backlog_max_writes_per_event, 8ll*1024*1024, MEMORY_CONFIG, NULL, NULL),
|
||||
|
||||
/* Unsigned Long Long configs */
|
||||
createULongLongConfig("maxmemory", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->maxmemory, 0, MEMORY_CONFIG, NULL, updateMaxmemory),
|
||||
|
||||
+10
-6
@@ -1871,21 +1871,23 @@ int writeToClient(client *c, int handler_installed) {
|
||||
|
||||
while (clientHasPendingReplies(c)) {
|
||||
long long repl_end_idx = getReplIndexFromOffset(c->repl_end_off);
|
||||
long long repl_max_writes = g_pserver->repl_backlog_max_writes_per_event > 0 ? g_pserver->repl_backlog_max_writes_per_event : LLONG_MAX;
|
||||
|
||||
serverAssert(c->repl_curr_off != -1);
|
||||
|
||||
if (c->repl_curr_off != c->repl_end_off){
|
||||
if (c->repl_curr_off != c->repl_end_off) {
|
||||
long long repl_curr_idx = getReplIndexFromOffset(c->repl_curr_off);
|
||||
long long nwritten2ndStage = 0; /* How much was written from the start of the replication backlog
|
||||
* in the event of a wrap around write */
|
||||
/* normal case with no wrap around */
|
||||
if (repl_end_idx >= repl_curr_idx){
|
||||
nwritten = connWrite(c->conn, g_pserver->repl_backlog + repl_curr_idx, repl_end_idx - repl_curr_idx);
|
||||
if (repl_end_idx >= repl_curr_idx) {
|
||||
nwritten = connWrite(c->conn, g_pserver->repl_backlog + repl_curr_idx, std::min(repl_max_writes, repl_end_idx - repl_curr_idx));
|
||||
/* wrap around case */
|
||||
} else {
|
||||
nwritten = connWrite(c->conn, g_pserver->repl_backlog + repl_curr_idx, g_pserver->repl_backlog_size - repl_curr_idx);
|
||||
nwritten = connWrite(c->conn, g_pserver->repl_backlog + repl_curr_idx, std::min(repl_max_writes, g_pserver->repl_backlog_size - repl_curr_idx));
|
||||
/* only attempt wrapping if we write the correct number of bytes */
|
||||
if (nwritten == g_pserver->repl_backlog_size - repl_curr_idx){
|
||||
nwritten2ndStage = connWrite(c->conn, g_pserver->repl_backlog, repl_end_idx);
|
||||
nwritten2ndStage = connWrite(c->conn, g_pserver->repl_backlog, std::min(repl_max_writes - nwritten, repl_end_idx));
|
||||
if (nwritten2ndStage != -1)
|
||||
nwritten += nwritten2ndStage;
|
||||
}
|
||||
@@ -1902,6 +1904,8 @@ int writeToClient(client *c, int handler_installed) {
|
||||
if (nwritten2ndStage == -1) nwritten = -1;
|
||||
if (nwritten == -1)
|
||||
break;
|
||||
|
||||
if (totwritten > g_pserver->repl_backlog_max_writes_per_event) break;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
@@ -1972,7 +1976,7 @@ int writeToClient(client *c, int handler_installed) {
|
||||
if (nwritten == -1) {
|
||||
if (connGetState(c->conn) != CONN_STATE_CONNECTED) {
|
||||
serverLog(LL_VERBOSE,
|
||||
"Error writing to client: %s", connGetLastError(c->conn));
|
||||
"Error writing to client: %s (totwritten: %zd)", connGetLastError(c->conn), totwritten);
|
||||
freeClientAsync(c);
|
||||
|
||||
return C_ERR;
|
||||
|
||||
@@ -2564,6 +2564,7 @@ struct redisServer {
|
||||
int repl_diskless_sync_delay; /* Delay to start a diskless repl BGSAVE. */
|
||||
std::atomic <long long> repl_lowest_off; /* The lowest offset amongst all replicas
|
||||
-1 if there are no replicas */
|
||||
long long repl_backlog_max_writes_per_event; /* Caps the output buffer sent to the replica at once */
|
||||
/* Replication (replica) */
|
||||
list *masters;
|
||||
int enable_multimaster;
|
||||
|
||||
@@ -1071,6 +1071,12 @@ static int connTLSWrite(connection *conn_, const void *data, size_t data_len) {
|
||||
|
||||
if (conn->c.state != CONN_STATE_CONNECTED) return -1;
|
||||
ERR_clear_error();
|
||||
|
||||
if (data_len > std::numeric_limits<int>::max()) {
|
||||
// OpenSSL expects length to be 32-bit int
|
||||
data_len = std::numeric_limits<int>::max();
|
||||
}
|
||||
|
||||
ret = SSL_write(conn->ssl, data, data_len);
|
||||
|
||||
if (ret <= 0) {
|
||||
|
||||
Reference in new issue
Block a user