TCP_NODELAY after SYNC: changes to the implementation.

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.

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);
 }
 
 

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);
 

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) {

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. */
 

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)
         {

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 */

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 */

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;