From b70b459b0eeea3158984ad89345a461cfe9f2818 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 31 Jan 2013 11:14:15 +0100 Subject: [PATCH] TCP_NODELAY after SYNC: changes to the implementation. --- redis.conf | 15 +++++++++++++++ src/anet.c | 12 ++++++------ src/anet.h | 4 ++-- src/cluster.c | 2 +- src/config.c | 16 ++++++++++------ src/networking.c | 2 +- src/redis.c | 2 +- src/redis.h | 2 +- src/replication.c | 11 +++-------- 9 files changed, 40 insertions(+), 26 deletions(-) diff --git a/redis.conf b/redis.conf index f6fe6506..b71a494e 100644 --- a/redis.conf +++ b/redis.conf @@ -196,6 +196,21 @@ slave-read-only yes # # repl-timeout 60 +# Disable TCP_NODELAY on the slave socket after SYNC? +# +# If you select "yes" Redis will use a smaller number of TCP packets and +# less bandwidth to send data to slaves. But this can add a delay for +# the data to appear on the slave side, up to 40 milliseconds with +# Linux kernels using a default configuration. +# +# If you select "no" the delay for data to appear on the slave side will +# be reduced but more bandwidth will be used for replication. +# +# By default we optimize for low latency, but in very high traffic conditions +# or when the master and slaves are many hops away, turning this to "yes" may +# be a good idea. +repl-disable-tcp-nodelay no + # The slave priority is an integer number published by Redis in the INFO output. # It is used by Redis Sentinel in order to select a slave to promote into a # master if the master is no longer working correctly. diff --git a/src/anet.c b/src/anet.c index d002cb31..3f037dca 100644 --- a/src/anet.c +++ b/src/anet.c @@ -75,9 +75,9 @@ int anetNonBlock(char *err, int fd) return ANET_OK; } -static int _anetTcpNoDelay(char *err, int fd, int yes) +static int anetSetTcpNoDelay(char *err, int fd, int val) { - if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &yes, sizeof(yes)) == -1) + if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)) == -1) { anetSetError(err, "setsockopt TCP_NODELAY: %s", strerror(errno)); return ANET_ERR; @@ -85,14 +85,14 @@ static int _anetTcpNoDelay(char *err, int fd, int yes) return ANET_OK; } -int anetTcpNoDelay(char *err, int fd) +int anetEnableTcpNoDelay(char *err, int fd) { - return _anetTcpNoDelay(err, fd, 1); + return anetSetTcpNoDelay(err, fd, 1); } -int anetTcpNoDelayOff(char *err, int fd) +int anetDisableTcpNoDelay(char *err, int fd) { - return _anetTcpNoDelay(err, fd, 0); + return anetSetTcpNoDelay(err, fd, 0); } diff --git a/src/anet.h b/src/anet.h index 56ae5057..dbf36dbd 100644 --- a/src/anet.h +++ b/src/anet.h @@ -51,8 +51,8 @@ int anetTcpAccept(char *err, int serversock, char *ip, int *port); int anetUnixAccept(char *err, int serversock); int anetWrite(int fd, char *buf, int count); int anetNonBlock(char *err, int fd); -int anetTcpNoDelay(char *err, int fd); -int anetTcpNoDelayOff(char *err, int fd); +int anetEnableTcpNoDelay(char *err, int fd); +int anetDisableTcpNoDelay(char *err, int fd); int anetTcpKeepAlive(char *err, int fd); int anetPeerToString(int fd, char *ip, int *port); diff --git a/src/cluster.c b/src/cluster.c index 24382b69..6bc16555 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1685,7 +1685,7 @@ int migrateGetSocket(redisClient *c, robj *host, robj *port, long timeout) { server.neterr); return -1; } - anetTcpNoDelay(server.neterr,fd); + anetEnableTcpNoDelay(server.neterr,fd); /* Check if it connects within the specified timeout. */ if ((aeWait(fd,AE_WRITABLE,timeout) & AE_WRITABLE) == 0) { diff --git a/src/config.c b/src/config.c index f4c45fc3..37cc138b 100644 --- a/src/config.c +++ b/src/config.c @@ -230,6 +230,10 @@ void loadServerConfigFromString(char *config) { err = "repl-timeout must be 1 or greater"; goto loaderr; } + } else if (!strcasecmp(argv[0],"repl-disable-tcp-nodelay") && argc==2) { + if ((server.repl_disable_tcp_nodelay = yesnotoi(argv[1])) == -1) { + err = "argument must be 'yes' or 'no'"; goto loaderr; + } } else if (!strcasecmp(argv[0],"masterauth") && argc == 2) { server.masterauth = zstrdup(argv[1]); } else if (!strcasecmp(argv[0],"slave-serve-stale-data") && argc == 2) { @@ -389,8 +393,6 @@ void loadServerConfigFromString(char *config) { if ((server.stop_writes_on_bgsave_err = yesnotoi(argv[1])) == -1) { err = "argument must be 'yes' or 'no'"; goto loaderr; } - } else if (!strcasecmp(argv[0],"slave-tcp-nodelay-off") && argc == 2) { - server.slave_tcp_nodelay_off = atoi(argv[1]); } else if (!strcasecmp(argv[0],"slave-priority") && argc == 2) { server.slave_priority = atoi(argv[1]); } else if (!strcasecmp(argv[0],"notify-keyspace-events") && argc == 2) { @@ -724,10 +726,11 @@ void configSetCommand(redisClient *c) { if (flags == -1) goto badfmt; server.notify_keyspace_events = flags; - } else if (!strcasecmp(c->argv[2]->ptr,"slave-tcp-nodelay-off")) { - if (getLongLongFromObject(o,&ll) == REDIS_ERR ) goto badfmt; + } else if (!strcasecmp(c->argv[2]->ptr,"repl-disable-tcp-nodelay")) { + int yn = yesnotoi(o->ptr); - server.slave_tcp_nodelay_off = ll; + if (yn == -1) goto badfmt; + server.repl_disable_tcp_nodelay = yn; } else if (!strcasecmp(c->argv[2]->ptr,"slave-priority")) { if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll <= 0) goto badfmt; @@ -821,7 +824,6 @@ void configGetCommand(redisClient *c) { config_get_numerical_field("repl-timeout",server.repl_timeout); config_get_numerical_field("maxclients",server.maxclients); config_get_numerical_field("watchdog-period",server.watchdog_period); - config_get_numerical_field("slave-tcp-nodelay-off",server.slave_tcp_nodelay_off); config_get_numerical_field("slave-priority",server.slave_priority); config_get_numerical_field("hz",server.hz); @@ -838,6 +840,8 @@ void configGetCommand(redisClient *c) { config_get_bool_field("rdbcompression", server.rdb_compression); config_get_bool_field("rdbchecksum", server.rdb_checksum); config_get_bool_field("activerehashing", server.activerehashing); + config_get_bool_field("repl-disable-tcp-nodelay", + server.repl_disable_tcp_nodelay); /* Everything we can't handle with macros follows. */ diff --git a/src/networking.c b/src/networking.c index 0853ad92..a7d14b49 100644 --- a/src/networking.c +++ b/src/networking.c @@ -58,7 +58,7 @@ redisClient *createClient(int fd) { * contexts (for instance a Lua script) we need a non connected client. */ if (fd != -1) { anetNonBlock(NULL,fd); - anetTcpNoDelay(NULL,fd); + anetEnableTcpNoDelay(NULL,fd); if (aeCreateFileEvent(server.el,fd,AE_READABLE, readQueryFromClient, c) == AE_ERR) { diff --git a/src/redis.c b/src/redis.c index c0c3af38..14bb9746 100644 --- a/src/redis.c +++ b/src/redis.c @@ -1200,7 +1200,7 @@ void initServerConfig() { server.repl_serve_stale_data = 1; server.repl_slave_ro = 1; server.repl_down_since = time(NULL); - server.slave_tcp_nodelay_off = 1; + server.repl_disable_tcp_nodelay = 0; server.slave_priority = REDIS_DEFAULT_SLAVE_PRIORITY; /* Client output buffer limits */ diff --git a/src/redis.h b/src/redis.h index b4955d07..a0f86974 100644 --- a/src/redis.h +++ b/src/redis.h @@ -763,7 +763,7 @@ struct redisServer { int repl_serve_stale_data; /* Serve stale data when link is down? */ int repl_slave_ro; /* Slave is read only? */ time_t repl_down_since; /* Unix time at which link with master went down */ - int slave_tcp_nodelay_off; /* turn off slave's tcp nodelay */ + int repl_disable_tcp_nodelay; /* Disable TCP_NODELAY after SYNC? */ int slave_priority; /* Reported in INFO and used by Sentinel. */ /* Limits */ unsigned int maxclients; /* Max number of simultaneous clients */ diff --git a/src/replication.c b/src/replication.c index fffac0e1..3819c885 100644 --- a/src/replication.c +++ b/src/replication.c @@ -118,14 +118,6 @@ void syncCommand(redisClient *c) { /* ignore SYNC if already slave or in monitor mode */ if (c->flags & REDIS_SLAVE) return; - if (server.slave_tcp_nodelay_off) { - redisLog(REDIS_NOTICE, "Turning off slave's :%d TCP NODELAY SETTING", c->fd); - char err[1024]; - if (anetTcpNoDelayOff(err, c->fd) == ANET_ERR) - redisLog(REDIS_WARNING, - "Can't turn off %d 's tcp nodelay setting: %s", c->fd, err); - } - /* Refuse SYNC requests if we are a slave but the link with our master * is not ok... */ if (server.masterhost && server.repl_state != REDIS_REPL_CONNECTED) { @@ -180,6 +172,9 @@ void syncCommand(redisClient *c) { } c->replstate = REDIS_REPL_WAIT_BGSAVE_END; } + + if (server.repl_disable_tcp_nodelay) + anetDisableTcpNoDelay(NULL, c->fd); /* Non critical if it fails. */ c->repldbfd = -1; c->flags |= REDIS_SLAVE; c->slaveseldb = 0;