diff options
author | antirez <antirez@gmail.com> | 2015-08-04 16:56:00 +0200 |
---|---|---|
committer | antirez <antirez@gmail.com> | 2015-08-04 17:06:10 +0200 |
commit | 292fec058a32323d5aa52dddfa86be280e29fe65 (patch) | |
tree | f728ed2b4687d008f4e03ae908359922286eb5b5 /src/syncio.c | |
parent | d1ff328170a161fc002e47954e5dd0e0989d2ce9 (diff) | |
download | redis-292fec058a32323d5aa52dddfa86be280e29fe65.tar.gz |
PSYNC initial offset fix.
This commit attempts to fix a bug involving PSYNC and diskless
replication (currently experimental) found by Yuval Inbar from Redis Labs
and that was later found to have even more far reaching effects (the bug also
exists when diskstore is off).
The gist of the bug is that, a Redis master replies with +FULLRESYNC to
a PSYNC attempt that fails and requires a full resynchronization.
However, the baseline offset sent along with FULLRESYNC was always the
current master replication offset. This is not ok, because there are
many reasosn that may delay the RDB file creation. And... guess what,
the master offset we communicate must be the one of the time the RDB
was created. So for example:
1) When the BGSAVE for replication is delayed since there is one
already but is not good for replication.
2) When the BGSAVE is not needed as we attach one currently ongoing.
3) When because of diskless replication the BGSAVE is delayed.
In all the above cases the PSYNC reply is wrong and the slave may
reconnect later claiming to need a wrong offset: this may cause
data curruption later.
Diffstat (limited to 'src/syncio.c')
-rw-r--r-- | src/syncio.c | 13 |
1 files changed, 9 insertions, 4 deletions
diff --git a/src/syncio.c b/src/syncio.c index b2843d5fb..48e0a0b79 100644 --- a/src/syncio.c +++ b/src/syncio.c @@ -118,7 +118,9 @@ ssize_t syncRead(int fd, char *ptr, ssize_t size, long long timeout) { } /* Read a line making sure that every char will not require more than 'timeout' - * milliseconds to be read. + * milliseconds to be read. Empty newlines before the first non-empty line + * are ignored. This is useful because since Redis sometimes uses empty + * newlines in order to take the connection "alive". * * On success the number of bytes read is returned, otherwise -1. * On success the string is always correctly terminated with a 0 byte. */ @@ -131,9 +133,12 @@ ssize_t syncReadLine(int fd, char *ptr, ssize_t size, long long timeout) { if (syncRead(fd,&c,1,timeout) == -1) return -1; if (c == '\n') { - *ptr = '\0'; - if (nread && *(ptr-1) == '\r') *(ptr-1) = '\0'; - return nread; + /* Ignore empty lines, otherwise return to the caller. */ + if (nread != 0) { + *ptr = '\0'; + if (nread && *(ptr-1) == '\r') *(ptr-1) = '\0'; + return nread; + } } else { *ptr++ = c; *ptr = '\0'; |