From aa794ac06d196945941b6a226bf40d36d16cf9cf Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 13 Jan 2012 17:49:16 -0800 Subject: [PATCH] Don't expire keys when loading an RDB after a SYNC The cron is responsible for expiring keys. When keys are expired at load time, it is possible that the snapshot of a master node gets modified. This can in turn lead to inconsistencies in the data set. A more concrete example of this behavior follows. A user reported a slave that would show an monotonically increase input buffer length, shortly after completing a SYNC. Also, `INFO` output showed a single blocked client, which could only be the master link. Investigation showed that indeed the `BRPOP` command was fed by the master. This command can only end up in the stream of write operations when it did NOT block, and effectively executed `RPOP`. However, when the key involved in the `BRPOP` is expired BEFORE the command is executed, the client executing it will block. The client in this case, is the master link. --- src/rdb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index 7d11a113426..4a6318b53f2 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -993,8 +993,12 @@ int rdbLoad(char *filename) { if ((key = rdbLoadStringObject(fp)) == NULL) goto eoferr; /* Read value */ if ((val = rdbLoadObject(type,fp)) == NULL) goto eoferr; - /* Check if the key already expired */ - if (expiretime != -1 && expiretime < now) { + /* Check if the key already expired. This function is used when loading + * an RDB file from disk, either at startup, or when an RDB was + * received from the master. In the latter case, the master is + * responsible for key expiry. If we would expire keys here, the + * snapshot taken by the master may not be reflected on the slave. */ + if (server.masterhost == NULL && expiretime != -1 && expiretime < now) { decrRefCount(key); decrRefCount(val); continue;