hi , when slave just finish read the full rdb file from master, rename it, ant then emptyDb, rdbLoad(server.rdb_filename) . but if there just happened a BGSAVE operation, the server.repl_transfer_tmpfile file my be overwriten by rename(tmpfile,filename) in rdbSave . as a result , the data is corrupted !
You can find this bug by gdb the bgsave process and slave main process to perform the rename() call . this bug occured in both 2.4-2.8.8 ，may be in the newest branch.
suggestion : BGSAVE and SYNC cann't perform Simultaneously 。
Comment From: mattsta
This is related to #1560, but it's a different overall issue. Looks like each save should use a unique temporary file.
Right now Redis has 7 places where it creates custom temporary file names. We can replace them all with a real mkstemp temporary file interface:
matt@ununoctium:~/repos/redis/src% grep -n temp- *.c
aof.c:901: snprintf(tmpfile,256,"temp-rewriteaof-%d.aof", (int) getpid());
aof.c:1018: snprintf(tmpfile,256,"temp-rewriteaof-bg-%d.aof", (int) getpid());
aof.c:1073: snprintf(tmpfile,256,"temp-rewriteaof-bg-%d.aof", (int) childpid);
aof.c:1105: snprintf(tmpfile,256,"temp-rewriteaof-bg-%d.aof",
rdb.c:641: snprintf(tmpfile,256,"temp-%d.rdb", (int) getpid());
rdb.c:766: snprintf(tmpfile,256,"temp-%d.rdb", (int) childpid);
replication.c:1161: "temp-%d.%ld.rdb",(int)server.unixtime,(long int)getpid());
Comment From: kulv2012
@mattsta , use mkstemp cann't fix this server.rdb_filename overwriten bug. no consider any attacks from #1563 , when redis slave doing sync, it relays the : 1. write rdb file, 2. read the above file.
bug when BGSAVE happen， the writed file was overwriten by bgsave process .. change tempfile cann't fix this race condition. so maybe the possible solution is that : [ we donn't allow any server.rdb_filename change durring slave sync . ]
Comment From: antirez
Hello @kulv2012, let's check if I'm understanding correctly the bug you described: - Instance is a slave. - It is resynchronizing with its master, so sent SYNC, and is receiving data from its master. - At the same time, a BGSAVE is triggered (in the slave).
Now we can see that the bulk transfer from the master is saved in the temp file obtained with:
"temp-%d.%ld.rdb",(int)server.unixtime,(long int)getpid()
While the other BGSAVE file is saved in a temp file obtained with:
"temp-%d.rdb", (int) getpid()
So far so good, two different files. - At some point, the data transfer from the master is completed. - Similarly, at some point, the child writing the database on disk completed its task.
So we have the last two steps that will both try to replace their temp file name with the final destination of the RDB file (dump.rdb by default).
If I understand correctly, the bug you described happens when the the BGSAVE temp file replaces the dump.rdb file in the worst moment, which is: - 1) rename(temp_file, dump_file); - 2) load(dump_file)
If between 1 and 2, the BGSAVE replaces the dump_file with the content of the BGSAVE operation (which is old data), we continue the replication process but we actually loaded the wrong old data.
Am I correct?
Salvatore
Comment From: hgqislub
Has this bug been fixed in 3.0?
Comment From: kulv2012
"If between 1 and 2, the BGSAVE replaces the dump_file with the content of the BGSAVE operation (which is old data), we continue the replication process but we actually loaded the wrong old data."
@antirez you are right, I mean it and tested it by gdb hang on the related lines. sorry for reply so late....
Comment From: kulv2012
@antirez @hgqislub it's not fixed in 3.0 and current unstable branch yet。 just now I have test it in the unstable branch by the following step. 1. run two instance: cd /home/wuhaiwen/redis && ./bin/redis-server conf/redis.conf --port 8379 cd /home/wuhaiwen/slave_redis && ./bin/redis-server-slave conf/redis.conf --port 8380 1. fill 1GB data to slave_redis, so as to have time to gdb hang on the forked bgsave process. 2. gdb the redis-server-slave process, break on readSyncBulkPayload rename line below (name A), and continue it . 1017 if (rename(server.repl_transfer_tmpfile,server.rdb_filename) == -1) { 3. redis-cli -p 8380 connect to slave, send bgsave commend to it . and ps aux | grep redis , find the forked bgsave process, gdb hang on it quickly , break in rdbSave function rename line as below (name B) . if (rename(tmpfile,filename) == -1) { 4. use gdb step to let A run rename frist, and then B run rename to change the filename to slave bgsaved file... 5. thus cause slave data error。
Comment From: yjh18717167823
@antirez @kulv2012 Has this bug been fixed in 4.0?
Comment From: yz1509
A similar phenomenon appeared in redis 3.2.11, has this bug been fixed in 5.0?
85382:M 25 Jun 07:22:51.189 * Slave 0.0.0.47:16410 asks for synchronization
85382:M 25 Jun 07:22:51.191 * Full resync requested by slave 0.0.0.0:16410
85382:M 25 Jun 07:22:51.191 * Starting BGSAVE for SYNC with target: disk
85382:M 25 Jun 07:22:51.291 * Background saving started by pid 29152
85382:M 25 Jun 07:23:12.001 # Connection with slave client id #5 lost.
85382:M 25 Jun 07:23:30.024 # Connection with slave 0.0.0.0:16410 lost.
85382:M 25 Jun 07:23:30.249 # Connection with slave client id #4 lost.
85382:S 25 Jun 07:23:42.331 * SLAVE OF 0.0.0.0:16410 enabled (user request from 'id=2920745 addr=0.0.0.47:38311 fd=110 name=sentinel-75d89339-cmd age=14 idle=0 flags=x db=0 sub=0 psub=0 multi=3 qbuf=0 qbuf-free=32768 obl=36 oll=0 omem=0 events=r cmd=exec')
85382:S 25 Jun 07:23:42.336 # CONFIG REWRITE executed with success.
85382:S 25 Jun 07:23:42.466 * Connecting to MASTER 0.0.0.0:16410
85382:S 25 Jun 07:23:42.467 * MASTER <-> SLAVE sync started
85382:S 25 Jun 07:23:42.467 * Non blocking connect for SYNC fired the event.
85382:S 25 Jun 07:23:42.469 * Master replied to PING, replication can continue...
85382:S 25 Jun 07:23:42.471 * Partial resynchronization not possible (no cached master)
85382:S 25 Jun 07:23:42.472 * Full resync from master: c7cbc541fa293da79e3a1a761be9478666e5a899:4755317435008
85382:S 25 Jun 07:24:54.532 * MASTER <-> SLAVE sync: receiving 4186414038 bytes from master
85382:S 25 Jun 07:25:57.541 * MASTER <-> SLAVE sync: Flushing old data
29152:C 25 Jun 07:27:44.038 * DB saved on disk
29152:C 25 Jun 07:27:44.061 * RDB: 13874 MB of memory used by copy-on-write
85382:S 25 Jun 07:28:57.441 * MASTER <-> SLAVE sync: Loading DB in memory
85382:S 25 Jun 07:31:31.013 * MASTER <-> SLAVE sync: Finished with success
85382:S 25 Jun 07:31:31.013 * Background saving terminated with success
Comment From: kulv2012
seams not yeat
Comment From: oranagra
this is fixed since redis 4.0 see 98a64523c4 466c277b4f 4dc69497f5 closing the issue, let me know if you still see a problem.
Comment From: kulv2012
this is fixed since redis 4.0 see 98a6452 466c277 4dc6949 closing the issue, let me know if you still see a problem.
thanks