Skip to content

Commit 8acf849

Browse files
kostasrimkostasrim
authoredMar 15, 2025
chore: clean up dbslice (#4769)
* merge AddOrFindResult with ItAndUpdater * remove unused functions * add missing FiberAtomicGuards on atomic member functions
1 parent 464dd05 commit 8acf849

7 files changed

+41
-71
lines changed
 

‎src/server/bitops_family.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ class ElementAccess {
261261
EngineShard* shard_ = nullptr;
262262
mutable DbSlice::AutoUpdater post_updater_;
263263

264-
void SetFields(EngineShard* shard, DbSlice::AddOrFindResult res);
264+
void SetFields(EngineShard* shard, DbSlice::ItAndUpdater res);
265265

266266
public:
267267
ElementAccess(string_view key, const OpArgs& args) : key_{key}, context_{args.db_cntx} {
@@ -298,7 +298,7 @@ std::optional<bool> ElementAccess::Exists(EngineShard* shard) {
298298
return res.status() != OpStatus::KEY_NOTFOUND;
299299
}
300300

301-
void ElementAccess::SetFields(EngineShard* shard, DbSlice::AddOrFindResult res) {
301+
void ElementAccess::SetFields(EngineShard* shard, DbSlice::ItAndUpdater res) {
302302
element_iter_ = res.it;
303303
added_ = res.is_new;
304304
shard_ = shard;

‎src/server/db_slice.cc

+20-34
Original file line numberDiff line numberDiff line change
@@ -479,14 +479,6 @@ DbSlice::AutoUpdater::AutoUpdater(const Fields& fields) : fields_(fields) {
479479
fields_.orig_heap_size = fields.it->second.MallocUsed();
480480
}
481481

482-
DbSlice::AddOrFindResult& DbSlice::AddOrFindResult::operator=(ItAndUpdater&& o) {
483-
it = o.it;
484-
exp_it = o.exp_it;
485-
is_new = false;
486-
post_updater = std::move(o).post_updater;
487-
return *this;
488-
}
489-
490482
DbSlice::ItAndUpdater DbSlice::FindMutable(const Context& cntx, string_view key) {
491483
return std::move(FindMutableInternal(cntx, key, std::nullopt).value());
492484
}
@@ -626,12 +618,11 @@ auto DbSlice::FindInternal(const Context& cntx, string_view key, optional<unsign
626618
return res;
627619
}
628620

629-
OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFind(const Context& cntx, string_view key) {
621+
OpResult<DbSlice::ItAndUpdater> DbSlice::AddOrFind(const Context& cntx, string_view key) {
630622
return AddOrFindInternal(cntx, key);
631623
}
632624

633-
OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindInternal(const Context& cntx,
634-
string_view key) {
625+
OpResult<DbSlice::ItAndUpdater> DbSlice::AddOrFindInternal(const Context& cntx, string_view key) {
635626
DCHECK(IsDbValid(cntx.db_index));
636627

637628
DbTable& db = *db_arr_[cntx.db_index];
@@ -644,15 +635,15 @@ OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindInternal(const Context& cnt
644635

645636
// PreUpdate() might have caused a deletion of `it`
646637
if (res->it.IsOccupied()) {
647-
return DbSlice::AddOrFindResult{
638+
return ItAndUpdater{
648639
.it = it,
649640
.exp_it = exp_it,
650-
.is_new = false,
651641
.post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
652642
.db_slice = this,
653643
.db_ind = cntx.db_index,
654644
.it = it,
655-
.key = key})};
645+
.key = key}),
646+
.is_new = false};
656647
} else {
657648
res = OpStatus::KEY_NOTFOUND;
658649
}
@@ -760,15 +751,14 @@ OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindInternal(const Context& cnt
760751
db.slots_stats[sid].key_count += 1;
761752
}
762753

763-
return DbSlice::AddOrFindResult{
764-
.it = Iterator(it, StringOrView::FromView(key)),
765-
.exp_it = ExpIterator{},
766-
.is_new = true,
767-
.post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
768-
.db_slice = this,
769-
.db_ind = cntx.db_index,
770-
.it = Iterator(it, StringOrView::FromView(key)),
771-
.key = key})};
754+
return ItAndUpdater{.it = Iterator(it, StringOrView::FromView(key)),
755+
.exp_it = ExpIterator{},
756+
.post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
757+
.db_slice = this,
758+
.db_ind = cntx.db_index,
759+
.it = Iterator(it, StringOrView::FromView(key)),
760+
.key = key}),
761+
.is_new = true};
772762
}
773763

774764
void DbSlice::ActivateDb(DbIndex db_ind) {
@@ -1049,11 +1039,10 @@ OpResult<int64_t> DbSlice::UpdateExpire(const Context& cntx, Iterator prime_it,
10491039
}
10501040
}
10511041

1052-
OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrUpdateInternal(const Context& cntx,
1053-
std::string_view key,
1054-
PrimeValue obj,
1055-
uint64_t expire_at_ms,
1056-
bool force_update) {
1042+
OpResult<DbSlice::ItAndUpdater> DbSlice::AddOrUpdateInternal(const Context& cntx,
1043+
std::string_view key, PrimeValue obj,
1044+
uint64_t expire_at_ms,
1045+
bool force_update) {
10571046
DCHECK(!obj.IsRef());
10581047

10591048
auto op_result = AddOrFind(cntx, key);
@@ -1084,8 +1073,8 @@ OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrUpdateInternal(const Context& c
10841073
return op_result;
10851074
}
10861075

1087-
OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrUpdate(const Context& cntx, string_view key,
1088-
PrimeValue obj, uint64_t expire_at_ms) {
1076+
OpResult<DbSlice::ItAndUpdater> DbSlice::AddOrUpdate(const Context& cntx, string_view key,
1077+
PrimeValue obj, uint64_t expire_at_ms) {
10891078
return AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, true);
10901079
}
10911080

@@ -1545,6 +1534,7 @@ void DbSlice::SetNotifyKeyspaceEvents(std::string_view notify_keyspace_events) {
15451534
}
15461535

15471536
void DbSlice::QueueInvalidationTrackingMessageAtomic(std::string_view key) {
1537+
FiberAtomicGuard guard;
15481538
auto it = client_tracking_map_.find(key);
15491539
if (it == client_tracking_map_.end()) {
15501540
return;
@@ -1651,10 +1641,6 @@ size_t DbSlice::StopSampleKeys(DbIndex db_ind) {
16511641
return count;
16521642
}
16531643

1654-
void DbSlice::PerformDeletion(PrimeIterator del_it, DbTable* table) {
1655-
return PerformDeletion(Iterator::FromPrime(del_it), table);
1656-
}
1657-
16581644
void DbSlice::PerformDeletionAtomic(Iterator del_it, ExpIterator exp_it, DbTable* table) {
16591645
FiberAtomicGuard guard;
16601646
size_t table_before = table->table_memory();

‎src/server/db_slice.h

+8-22
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,7 @@ class DbSlice {
285285
Iterator it;
286286
ExpIterator exp_it;
287287
AutoUpdater post_updater;
288-
289-
bool IsValid() const {
290-
return !it.is_done();
291-
}
288+
bool is_new = false;
292289
};
293290

294291
ItAndUpdater FindMutable(const Context& cntx, std::string_view key);
@@ -304,20 +301,11 @@ class DbSlice {
304301
OpResult<ConstIterator> FindReadOnly(const Context& cntx, std::string_view key,
305302
unsigned req_obj_type) const;
306303

307-
struct AddOrFindResult {
308-
Iterator it;
309-
ExpIterator exp_it;
310-
bool is_new = false;
311-
AutoUpdater post_updater;
312-
313-
AddOrFindResult& operator=(ItAndUpdater&& o);
314-
};
315-
316-
OpResult<AddOrFindResult> AddOrFind(const Context& cntx, std::string_view key);
304+
OpResult<ItAndUpdater> AddOrFind(const Context& cntx, std::string_view key);
317305

318306
// Same as AddOrSkip, but overwrites in case entry exists.
319-
OpResult<AddOrFindResult> AddOrUpdate(const Context& cntx, std::string_view key, PrimeValue obj,
320-
uint64_t expire_at_ms);
307+
OpResult<ItAndUpdater> AddOrUpdate(const Context& cntx, std::string_view key, PrimeValue obj,
308+
uint64_t expire_at_ms);
321309

322310
// Adds a new entry. Requires: key does not exist in this slice.
323311
// Returns the iterator to the newly added entry.
@@ -552,9 +540,9 @@ class DbSlice {
552540

553541
bool DelEmptyPrimeValue(const Context& cntx, Iterator it);
554542

555-
OpResult<AddOrFindResult> AddOrUpdateInternal(const Context& cntx, std::string_view key,
556-
PrimeValue obj, uint64_t expire_at_ms,
557-
bool force_update);
543+
OpResult<ItAndUpdater> AddOrUpdateInternal(const Context& cntx, std::string_view key,
544+
PrimeValue obj, uint64_t expire_at_ms,
545+
bool force_update);
558546

559547
void FlushSlotsFb(const cluster::SlotSet& slot_ids);
560548
void FlushDbIndexes(const std::vector<DbIndex>& indexes);
@@ -568,9 +556,7 @@ class DbSlice {
568556
// Clear tiered storage entries for the specified indices.
569557
void ClearOffloadedEntries(absl::Span<const DbIndex> indices, const DbTableArray& db_arr);
570558

571-
//
572559
void PerformDeletionAtomic(Iterator del_it, ExpIterator exp_it, DbTable* table);
573-
void PerformDeletion(PrimeIterator del_it, DbTable* table);
574560

575561
// Queues invalidation message to the clients that are tracking the change to a key.
576562
void QueueInvalidationTrackingMessageAtomic(std::string_view key);
@@ -590,7 +576,7 @@ class DbSlice {
590576

591577
PrimeItAndExp ExpireIfNeeded(const Context& cntx, PrimeIterator it) const;
592578

593-
OpResult<AddOrFindResult> AddOrFindInternal(const Context& cntx, std::string_view key);
579+
OpResult<ItAndUpdater> AddOrFindInternal(const Context& cntx, std::string_view key);
594580

595581
OpResult<PrimeItAndExp> FindInternal(const Context& cntx, std::string_view key,
596582
std::optional<unsigned> req_obj_type,

‎src/server/generic_family.cc

+6-8
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,8 @@ class RdbRestoreValue : protected RdbLoaderBase {
157157
rdb_version_ = rdb_version;
158158
}
159159

160-
OpResult<DbSlice::AddOrFindResult> Add(string_view key, string_view payload,
161-
const DbContext& cntx, const RestoreArgs& args,
162-
DbSlice* db_slice);
160+
OpResult<DbSlice::ItAndUpdater> Add(string_view key, string_view payload, const DbContext& cntx,
161+
const RestoreArgs& args, DbSlice* db_slice);
163162

164163
private:
165164
std::optional<OpaqueObj> Parse(io::Source* source);
@@ -190,10 +189,9 @@ std::optional<RdbLoaderBase::OpaqueObj> RdbRestoreValue::Parse(io::Source* sourc
190189
return std::optional<OpaqueObj>(std::move(obj));
191190
}
192191

193-
OpResult<DbSlice::AddOrFindResult> RdbRestoreValue::Add(string_view key, string_view data,
194-
const DbContext& cntx,
195-
const RestoreArgs& args,
196-
DbSlice* db_slice) {
192+
OpResult<DbSlice::ItAndUpdater> RdbRestoreValue::Add(string_view key, string_view data,
193+
const DbContext& cntx, const RestoreArgs& args,
194+
DbSlice* db_slice) {
197195
InMemSource data_src(data);
198196
PrimeValue pv;
199197
bool first_parse = true;
@@ -715,7 +713,7 @@ OpStatus OpExpire(const OpArgs& op_args, string_view key, const DbSlice::ExpireP
715713
OpResult<vector<long>> OpFieldExpire(const OpArgs& op_args, string_view key, uint32_t ttl_sec,
716714
CmdArgList values) {
717715
auto& db_slice = op_args.GetDbSlice();
718-
auto [it, expire_it, auto_updater] = db_slice.FindMutable(op_args.db_cntx, key);
716+
auto [it, expire_it, auto_updater, is_new] = db_slice.FindMutable(op_args.db_cntx, key);
719717

720718
if (!IsValid(it) || (it->second.ObjType() != OBJ_SET && it->second.ObjType() != OBJ_HASH)) {
721719
std::vector<long> res(values.size(), -2);

‎src/server/json_family.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,8 @@ std::optional<JsonType> ShardJsonFromString(std::string_view input) {
328328
return dfly::JsonFromString(input, CompactObj::memory_resource());
329329
}
330330

331-
OpResult<DbSlice::AddOrFindResult> SetJson(const OpArgs& op_args, string_view key,
332-
string_view json_str) {
331+
OpResult<DbSlice::ItAndUpdater> SetJson(const OpArgs& op_args, string_view key,
332+
string_view json_str) {
333333
auto& db_slice = op_args.GetDbSlice();
334334

335335
auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
@@ -1330,7 +1330,7 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
13301330
}
13311331

13321332
JsonMemTracker mem_tracker;
1333-
OpResult<DbSlice::AddOrFindResult> st = SetJson(op_args, key, json_str);
1333+
auto st = SetJson(op_args, key, json_str);
13341334
RETURN_ON_BAD_STATUS(st);
13351335
mem_tracker.SetJsonSize(st->it->second, st->is_new);
13361336
return true;

‎src/server/list_family.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ OpResult<string> Peek(const OpArgs& op_args, string_view key, ListDir dir, bool
326326
OpResult<uint32_t> OpPush(const OpArgs& op_args, std::string_view key, ListDir dir,
327327
bool skip_notexist, facade::ArgRange vals, bool journal_rewrite) {
328328
EngineShard* es = op_args.shard;
329-
DbSlice::AddOrFindResult res;
329+
DbSlice::ItAndUpdater res;
330330

331331
if (skip_notexist) {
332332
auto tmp_res = op_args.GetDbSlice().FindMutable(op_args.db_cntx, key, OBJ_LIST);

‎src/server/stream_family.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,7 @@ OpResult<streamID> OpAdd(const OpArgs& op_args, string_view key, const AddOpts&
673673

674674
auto& db_slice = op_args.GetDbSlice();
675675

676-
DbSlice::AddOrFindResult add_res;
676+
DbSlice::ItAndUpdater add_res;
677677
if (opts.no_mkstream) {
678678
auto res_it = db_slice.FindMutable(op_args.db_cntx, key, OBJ_STREAM);
679679
RETURN_ON_BAD_STATUS(res_it);

0 commit comments

Comments
 (0)