Skip to content

Commit

Permalink
fix(cluster): Don't miss keys when migrating slots (#3218)
Browse files Browse the repository at this point in the history
In rare cases, the fuzzy cluster migration test detected missing keys.
It turns out that the missing keys were skipped at the source side due
to contention:
* The OnDbChange callback registered and got a `snapshot_id`
* It then blocked on a mutex, and could not add itself to the list of
  callbacks
* When the mutex was released, it registered, but it missed all changes
  that happened between registering (`snapshot_id`) and the moment it
  registered
  • Loading branch information
chakaz authored Jun 25, 2024
1 parent 847e2ed commit f28bd93
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/server/db_slice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1095,15 +1095,18 @@ void DbSlice::ExpireAllIfNeeded() {
}

uint64_t DbSlice::RegisterOnChange(ChangeCallback cb) {
uint64_t ver = NextVersion();

// TODO rewrite this logic to be more clear
// this mutex lock is needed to check that this method is not called simultaneously with
// change_cb_ calls and journal_slice::change_cb_arr_ calls.
// It can be unlocked anytime because DbSlice::RegisterOnChange
// and journal_slice::RegisterOnChange calls without preemption
std::lock_guard lk(cb_mu_);

uint64_t ver = NextVersion();
change_cb_.emplace_back(ver, std::move(cb));
DCHECK(std::is_sorted(change_cb_.begin(), change_cb_.end(),
[](auto& a, auto& b) { return a.first < b.first; }));

return ver;
}

Expand Down

0 comments on commit f28bd93

Please sign in to comment.