mirrors/dragonfly
1
0
Fork 0
mirror of https://github.com/dragonflydb/dragonfly synced 2024-11-22 15:44:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Fix inconsistency when deleting dash entries (#1734)

Browse Source 
Partially addresses #1730.

Beforehand we removed entries from mcflag only inside DbSlice::Del function
but we delete entries in other cases too (like expiration) and in this case we left
dangling entries in mcflag table. The fix adds the deletion of mcflag entries into PerformDeletion function.

It by itself does not explain the crash but
dangling entries cause UB (because the keys are references to non-existent PrimeKeys)
so maybe this somehow causes inconsistency where a key is not present in mcflags.

I am not sure I fixed the original issue but I surely fixed a corruption bug.
In addition, I removed the redundant check fail and added error logs to follow up on this later.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
Roman Gershman 2023-08-24 17:02:02 +03:00 committed by GitHub
parent b0718abdd1
commit e4ea868175
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/server/db_slice.cc
View File

@ -38,6 +38,13 @@ void PerformDeletion(PrimeIterator del_it, ExpireIterator exp_it, EngineShard* s
table->expire.Erase(exp_it);
}
if (del_it->second.HasFlag()) {
if (table->mcflag.Erase(del_it->first) == 0) {
LOG(ERROR) << "Internal error, inconsistent state, mcflag should be present but not found "
<< del_it->first.ToString();
}
}
DbTableStats& stats = table->stats;
const PrimeValue& pv = del_it->second;
if (pv.IsExternal()) {
@ -442,6 +449,7 @@ tuple<PrimeIterator, ExpireIterator, bool> DbSlice::AddOrFind2(const Context& cn
SlotId sid = ClusterConfig::KeySlot(key);
db.slots_stats[sid].key_count += 1;
}
return make_tuple(it, ExpireIterator{}, true);
}
@ -464,7 +472,11 @@ tuple<PrimeIterator, ExpireIterator, bool> DbSlice::AddOrFind2(const Context& cn
db.expire.Erase(expire_it);
if (existing->second.HasFlag()) {
db.mcflag.Erase(existing->first);
if (db.mcflag.Erase(existing->first) == 0) {
LOG(ERROR)
<< "Internal error, inconsistent state, mcflag should be present but not found "
<< existing->first.ToString();
}
}
// Keep the entry but reset the object.
@ -493,9 +505,6 @@ bool DbSlice::Del(DbIndex db_ind, PrimeIterator it) {
}
auto& db = db_arr_[db_ind];
if (it->second.HasFlag()) {
CHECK_EQ(1u, db->mcflag.Erase(it->first));
}
auto obj_type = it->second.ObjType();
if (doc_del_cb_ && (obj_type == OBJ_JSON || obj_type == OBJ_HASH)) {
@ -638,7 +647,10 @@ bool DbSlice::UpdateExpire(DbIndex db_ind, PrimeIterator it, uint64_t at) {
void DbSlice::SetMCFlag(DbIndex db_ind, PrimeKey key, uint32_t flag) {
auto& db = *db_arr_[db_ind];
if (flag == 0) {
db.mcflag.Erase(key);
if (db.mcflag.Erase(key) == 0) {
LOG(ERROR) << "Internal error, inconsistent state, mcflag should be present but not found "
<< key.ToString();
}
} else {
auto [it, inserted] = db.mcflag.Insert(std::move(key), flag);
if (!inserted)
@ -649,7 +661,12 @@ void DbSlice::SetMCFlag(DbIndex db_ind, PrimeKey key, uint32_t flag) {
uint32_t DbSlice::GetMCFlag(DbIndex db_ind, const PrimeKey& key) const {
auto& db = *db_arr_[db_ind];
auto it = db.mcflag.Find(key);
return it.is_done() ? 0 : it->second;
if (it.is_done()) {
LOG(ERROR) << "Internal error, inconsistent state, mcflag should be present but not found "
<< key.ToString();
return 0;
}
return it->second;
}
PrimeIterator DbSlice::AddNew(const Context& cntx, string_view key, PrimeValue obj,

5
src/server/string_family.cc
View File

@ -1340,7 +1340,10 @@ auto StringFamily::OpMGet(bool fetch_mcflag, bool fetch_mcver, const Transaction
dest.value = GetString(shard, it->second);
if (fetch_mcflag) {
dest.mc_flag = db_slice.GetMCFlag(t->GetDbIndex(), it->first);
if (it->second.HasFlag()) {
dest.mc_flag = db_slice.GetMCFlag(t->GetDbIndex(), it->first);
}
if (fetch_mcver) {
dest.mc_ver = it.GetVersion();
}