diff --git a/src/server/db_slice.cc b/src/server/db_slice.cc index cabbdf25de7a..1aa1d3f8ca1c 100644 --- a/src/server/db_slice.cc +++ b/src/server/db_slice.cc @@ -816,6 +816,11 @@ void DbSlice::Del(Context cntx, Iterator it) { PerformDeletion(it, db.get()); } +void DbSlice::DelMutable(Context cntx, ItAndUpdater it_updater) { + it_updater.post_updater.Run(); + Del(cntx, it_updater.it); +} + void DbSlice::FlushSlotsFb(const cluster::SlotSet& slot_ids) { VLOG(1) << "Start FlushSlotsFb"; // Slot deletion can take time as it traverses all the database, hence it runs in fiber. diff --git a/src/server/db_slice.h b/src/server/db_slice.h index 3a1933b57a9f..b34c99168620 100644 --- a/src/server/db_slice.h +++ b/src/server/db_slice.h @@ -328,6 +328,11 @@ class DbSlice { // Deletes the iterator. The iterator must be valid. void Del(Context cntx, Iterator it); + // Deletes a key after FindMutable(). Runs post_updater before deletion + // to update memory accounting while the key is still valid. + // Takes ownership of it_updater (pass by value with move semantics). + void DelMutable(Context cntx, ItAndUpdater it_updater); + constexpr static DbIndex kDbAll = 0xFFFF; // Flushes db_ind or all databases if kDbAll is passed diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index bb7e744d7dd6..b51b7025f266 100644 --- a/src/server/generic_family.cc +++ b/src/server/generic_family.cc @@ -458,8 +458,7 @@ OpStatus Renamer::DelSrc(Transaction* t, EngineShard* shard) { DVLOG(1) << "Rename: removing the key '" << src_key_; - res.post_updater.Run(); - db_slice.Del(t->GetDbContext(), it); + db_slice.DelMutable(t->GetDbContext(), std::move(res)); if (shard->journal()) { RecordJournal(t->GetOpArgs(shard), "DEL"sv, ArgSlice{src_key_}, 2); } @@ -480,8 +479,7 @@ OpStatus Renamer::DeserializeDest(Transaction* t, EngineShard* shard) { if (dest_found_) { DVLOG(1) << "Rename: deleting the destiny key '" << dest_key_; - dest_res.post_updater.Run(); - db_slice.Del(op_args.db_cntx, dest_res.it); + db_slice.DelMutable(op_args.db_cntx, std::move(dest_res)); } if (restore_args.Expired()) { @@ -588,8 +586,7 @@ OpStatus OpRestore(const OpArgs& op_args, std::string_view key, std::string_view if (restore_args.Replace()) { VLOG(1) << "restore command is running with replace, found old key '" << key << "' and removing it"; - res.post_updater.Run(); - db_slice.Del(op_args.db_cntx, res.it); + db_slice.DelMutable(op_args.db_cntx, std::move(res)); } else { // we are not allowed to replace it. return OpStatus::KEY_EXISTS; @@ -938,14 +935,12 @@ OpResult OpRen(const OpArgs& op_args, string_view from_key, string_view to to_res.it->first.SetSticky(sticky); to_res.post_updater.Run(); - from_res.post_updater.Run(); - db_slice.Del(op_args.db_cntx, from_res.it); + db_slice.DelMutable(op_args.db_cntx, std::move(from_res)); } else { // Here we first delete from_it because AddNew below could invalidate from_it. // On the other hand, AddNew does not rely on the iterators - this is why we keep // the value in `from_obj`. - from_res.post_updater.Run(); - db_slice.Del(op_args.db_cntx, from_res.it); + db_slice.DelMutable(op_args.db_cntx, std::move(from_res)); auto op_result = db_slice.AddNew(op_args.db_cntx, to_key, std::move(from_obj), exp_ts); RETURN_ON_BAD_STATUS(op_result); to_res = std::move(*op_result); diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc index a59a1bd6d2a1..20b76bc079d9 100644 --- a/src/server/hset_family.cc +++ b/src/server/hset_family.cc @@ -281,8 +281,7 @@ struct KeyCleanup { void DeleteKey(DbSlice& db_slice, const OpArgs& op_args, std::string_view key) { if (auto del_it = db_slice.FindMutable(op_args.db_cntx, key, OBJ_HASH); del_it) { - del_it->post_updater.Run(); - db_slice.Del(op_args.db_cntx, del_it->it); + db_slice.DelMutable(op_args.db_cntx, std::move(*del_it)); if (op_args.shard->journal()) { RecordJournal(op_args, "DEL"sv, {key}); } @@ -1166,8 +1165,7 @@ void HSetFamily::HRandField(CmdArgList args, const CommandContext& cmd_cntx) { if (string_map->Empty()) { // Can happen if we use a TTL on hash members. auto res_it = db_slice.FindMutable(db_context, key, OBJ_HASH); if (res_it) { - res_it->post_updater.Run(); - db_slice.Del(db_context, res_it->it); + db_slice.DelMutable(db_context, std::move(*res_it)); } return facade::OpStatus::KEY_NOTFOUND; } diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 1eae9df12e21..9abab30147ca 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -1117,8 +1117,7 @@ OpResult OpDel(const OpArgs& op_args, string_view key, string_view path, RETURN_ON_BAD_STATUS(res_it); if (IsValid(res_it->it)) { - res_it->post_updater.Run(); - db_slice.Del(op_args.db_cntx, res_it->it); + db_slice.DelMutable(op_args.db_cntx, std::move(*res_it)); return 1; } return 0; diff --git a/src/server/set_family.cc b/src/server/set_family.cc index feee9dae7689..c46e5543d072 100644 --- a/src/server/set_family.cc +++ b/src/server/set_family.cc @@ -483,8 +483,7 @@ OpResult OpAdd(const OpArgs& op_args, std::string_view key, const NewE if (overwrite && (vals_it.begin() == vals_it.end())) { auto res_it = db_slice.FindMutable(op_args.db_cntx, key, OBJ_SET); if (res_it) { - res_it->post_updater.Run(); - db_slice.Del(op_args.db_cntx, res_it->it); + db_slice.DelMutable(op_args.db_cntx, std::move(*res_it)); if (journal_update && op_args.shard->journal()) { RecordJournal(op_args, "DEL"sv, ArgSlice{key}); } @@ -927,8 +926,7 @@ OpResult OpPop(const OpArgs& op_args, string_view key, unsigned count }); // Delete the set as it is now empty - find_res->post_updater.Run(); - db_slice.Del(op_args.db_cntx, find_res->it); + db_slice.DelMutable(op_args.db_cntx, std::move(*find_res)); // Replicate as DEL. if (op_args.shard->journal()) { diff --git a/src/server/string_family.cc b/src/server/string_family.cc index 197ac312643e..a80c8af8ef7e 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -1149,8 +1149,7 @@ void StringFamily::GetDel(CmdArgList args, const CommandContext& cmnd_cntx) { return it_res.status(); auto value = ReadString(tx->GetDbIndex(), key, it_res->it->second, es); - it_res->post_updater.Run(); // Run manually before delete - db_slice.Del(tx->GetDbContext(), it_res->it); + db_slice.DelMutable(tx->GetDbContext(), std::move(*it_res)); return value; }; diff --git a/src/server/zset_family.cc b/src/server/zset_family.cc index 99f8d24e688f..4bdb5a558976 100644 --- a/src/server/zset_family.cc +++ b/src/server/zset_family.cc @@ -1969,8 +1969,7 @@ OpResult ZSetFamily::OpAdd(const OpArgs& op_args, if (zparams.override && members.empty()) { auto res_it = db_slice.FindMutable(op_args.db_cntx, key, OBJ_ZSET); if (res_it && IsValid(res_it->it)) { - res_it->post_updater.Run(); - db_slice.Del(op_args.db_cntx, res_it->it); + db_slice.DelMutable(op_args.db_cntx, std::move(*res_it)); if (zparams.journal_update && op_args.shard->journal()) { RecordJournal(op_args, "DEL"sv, ArgSlice{key}); }