search

apache / kvrocks

Apache Kvrocks is a distributed key value NoSQL database that uses RocksDB as storage engine and is compatible with Redis protocol.
https://kvrocks.apache.org/
Apache License 2.0
3.47k stars 450 forks source link

Data inconsistency in master-slave replication on certain situation #346

Closed ChrisZMF closed 3 years ago

ChrisZMF commented 3 years ago

Describe the bug If we run kvrocks in master-slave structure, the data consistency may not be guaranteed on certain situation. Consider this situation, after all master data have been replicate to slave, we write a new key which will increase the sequence of rocksdb more than 20, then if we stop writing, this new key will not be copied to slave. The key will not be sent，util new data with sequence increment less than 20 or the length of master-slave replication queue exceeds 20. This situation will cause data inconsistency.

To Reproduce Steps to reproduce the behavior:

  1. Start two kvrocks servers and establish master-slave replication relationship.
  2. After replication, write a hash key with more than 20 f-v pairs, like hmset hkey f1 v1 f2 v2 ....... f21 v21, then stop writing.
  3. Check replication info on master and slave, then the following result will be gotten: image In above picture, master info is on left side, slave is on right side. We can get that offset=69,lag=0 on master and slave_repl_offset:47 on slave. offset represents the slave's sequence recorded on master which is 69, and lag=0 represents there is no lag between master and slave. In other words, from the perspective of master, slave and master are synchronized. But, on slave‘s perspective, slave_repl_offset is 47 which means slave lags behind master, it is not synchronized with master.
  4. Check keys on both master and slave, we will get this: image Also, left side is mater and right side is slave in above picture. We can see that the new key hkey is not copied to slave.

Expected behavior On slave, slave_repl_offset should be 69 too, and key hkey should exist.

Reason analysis In FeedSlaveThread::loop(), if (... || latest_seq - batch.sequence <= 20 || batch_list.size() >= 20) is adopted to improve data transmission efficiency in incremental replication phase. But, after all data being synchronized, latest_seq - batch.sequence <= 20 will cause the bug described above

ShooterIT commented 3 years ago

let me fix that, and i want to lightly refactor other somethings