From cf991deb030b3f547be715b1223a01df19f61591 Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Tue, 2 Aug 2022 11:02:51 +0300 Subject: [PATCH] chore(server): add updateval_amount statistic. This statistic helps understanding how much Dragonfly memory grows via updating the existing value vs the new ones. Signed-off-by: Roman Gershman --- src/server/db_slice.cc | 26 +++++++++------- src/server/db_slice.h | 6 +++- src/server/list_family.cc | 13 ++++---- src/server/server_family.cc | 1 + src/server/set_family.cc | 16 +++++----- src/server/string_family.cc | 59 ++++++++++++++++--------------------- src/server/table.cc | 3 +- src/server/table.h | 3 ++ 8 files changed, 67 insertions(+), 60 deletions(-) diff --git a/src/server/db_slice.cc b/src/server/db_slice.cc index 15a2a55..ca10452 100644 --- a/src/server/db_slice.cc +++ b/src/server/db_slice.cc @@ -142,7 +142,7 @@ unsigned PrimeEvictionPolicy::Evict(const PrimeTable::HotspotBuckets& eb, PrimeT DbStats& DbStats::operator+=(const DbStats& o) { constexpr size_t kDbSz = sizeof(DbStats); - static_assert(kDbSz == 88); + static_assert(kDbSz == 96); DbTableStats::operator+=(o); @@ -287,8 +287,8 @@ pair DbSlice::AddOrFind(DbIndex db_index, string_view key) return make_pair(get<0>(res), get<2>(res)); } -tuple DbSlice::AddOrFind2(DbIndex db_index, - std::string_view key) noexcept(false) { +tuple DbSlice::AddOrFind2( + DbIndex db_index, string_view key) noexcept(false) { DCHECK(IsDbValid(db_index)); auto& db = db_arr_[db_index]; @@ -497,7 +497,7 @@ pair DbSlice::AddOrFind(DbIndex db_ind, string_view key, Pr auto& it = res.first; it->second = std::move(obj); - PostUpdate(db_ind, it); + PostUpdate(db_ind, it, false); if (expire_at_ms) { it->second.SetExpire(true); @@ -582,15 +582,16 @@ bool DbSlice::CheckLock(IntentLock::Mode mode, const KeyLockArgs& lock_args) con } void DbSlice::PreUpdate(DbIndex db_ind, PrimeIterator it) { - auto& db = db_arr_[db_ind]; for (const auto& ccb : change_cb_) { ccb.second(db_ind, ChangeReq{it}); } size_t value_heap_size = it->second.MallocUsed(); - db->stats.obj_memory_usage -= value_heap_size; + auto* stats = MutableStats(db_ind); + stats->obj_memory_usage -= value_heap_size; + stats->update_value_amount -= value_heap_size; if (it->second.ObjType() == OBJ_STRING) { - db->stats.strval_memory_usage -= value_heap_size; + stats->strval_memory_usage -= value_heap_size; if (it->second.IsExternal()) { TieredStorage* tiered = shard_owner()->tiered_storage(); auto [offset, size] = it->second.GetExternalPtr(); @@ -602,12 +603,15 @@ void DbSlice::PreUpdate(DbIndex db_ind, PrimeIterator it) { it.SetVersion(NextVersion()); } -void DbSlice::PostUpdate(DbIndex db_ind, PrimeIterator it) { - auto& db = db_arr_[db_ind]; +void DbSlice::PostUpdate(DbIndex db_ind, PrimeIterator it, bool existing) { + DbTableStats* stats = MutableStats(db_ind); + size_t value_heap_size = it->second.MallocUsed(); - db->stats.obj_memory_usage += value_heap_size; + stats->obj_memory_usage += value_heap_size; if (it->second.ObjType() == OBJ_STRING) - db->stats.strval_memory_usage += value_heap_size; + stats->strval_memory_usage += value_heap_size; + if (existing) + stats->update_value_amount += value_heap_size; } pair DbSlice::ExpireIfNeeded(DbIndex db_ind, diff --git a/src/server/db_slice.h b/src/server/db_slice.h index 5fc74ff..362d6c4 100644 --- a/src/server/db_slice.h +++ b/src/server/db_slice.h @@ -99,6 +99,10 @@ class DbSlice { memory_budget_ = budget; } + ssize_t memory_budget() const { + return memory_budget_; + } + // returns absolute time of the expiration. time_t ExpireTime(ExpireIterator it) const { return it.is_done() ? 0 : expire_base_[0] + it->second.duration_ms(); @@ -198,7 +202,7 @@ class DbSlice { // Callback functions called upon writing to the existing key. void PreUpdate(DbIndex db_ind, PrimeIterator it); - void PostUpdate(DbIndex db_ind, PrimeIterator it); + void PostUpdate(DbIndex db_ind, PrimeIterator it, bool existing_entry = true); DbTableStats* MutableStats(DbIndex db_ind) { return &db_arr_[db_ind]->stats; diff --git a/src/server/list_family.cc b/src/server/list_family.cc index aea3787..3a5d841 100644 --- a/src/server/list_family.cc +++ b/src/server/list_family.cc @@ -306,16 +306,15 @@ OpResult OpRPopLPushSingleShard(const OpArgs& op_args, string_view src, } quicklist* dest_ql = nullptr; - pair res; + PrimeIterator dest_it; + bool new_key = false; try { - res = db_slice.AddOrFind(op_args.db_ind, dest); + tie(dest_it, new_key) = db_slice.AddOrFind(op_args.db_ind, dest); } catch (bad_alloc&) { return OpStatus::OUT_OF_MEMORY; } - PrimeIterator& dest_it = res.first; - - if (res.second) { + if (new_key) { robj* obj = createQuicklistObject(); dest_ql = (quicklist*)obj->ptr; quicklistSetOptions(dest_ql, GetFlag(FLAGS_list_max_listpack_size), @@ -338,7 +337,7 @@ OpResult OpRPopLPushSingleShard(const OpArgs& op_args, string_view src, quicklistPushHead(dest_ql, val.data(), val.size()); db_slice.PostUpdate(op_args.db_ind, src_it); - db_slice.PostUpdate(op_args.db_ind, dest_it); + db_slice.PostUpdate(op_args.db_ind, dest_it, !new_key); if (quicklistCount(src_ql) == 0) { CHECK(db_slice.Del(op_args.db_ind, src_it)); @@ -419,7 +418,7 @@ OpResult OpPush(const OpArgs& op_args, std::string_view key, ListDir d es->blocking_controller()->AwakeWatched(op_args.db_ind, key); } } else { - es->db_slice().PostUpdate(op_args.db_ind, it); + es->db_slice().PostUpdate(op_args.db_ind, it, true); } return quicklistCount(ql); diff --git a/src/server/server_family.cc b/src/server/server_family.cc index cd50f05..9dabf77 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -842,6 +842,7 @@ void ServerFamily::Info(CmdArgList args, ConnectionContext* cntx) { append("num_entries", total.key_count); append("inline_keys", total.inline_keys); append("strval_bytes", total.strval_memory_usage); + append("updateval_amount", total.update_value_amount); append("listpack_blobs", total.listpack_blob_cnt); append("listpack_bytes", total.listpack_bytes); append("small_string_bytes", m.small_string_bytes); diff --git a/src/server/set_family.cc b/src/server/set_family.cc index 5a1c2e4..909a303 100644 --- a/src/server/set_family.cc +++ b/src/server/set_family.cc @@ -311,29 +311,31 @@ OpResult OpAdd(const OpArgs& op_args, std::string_view key, ArgSlice v if (overwrite && vals.empty()) { auto it = db_slice.FindExt(op_args.db_ind, key).first; db_slice.Del(op_args.db_ind, it); + return 0; } - pair add_res; + PrimeIterator it; + bool new_key = false; try { - add_res = db_slice.AddOrFind(op_args.db_ind, key); + tie(it, new_key) = db_slice.AddOrFind(op_args.db_ind, key); } catch (bad_alloc& e) { return OpStatus::OUT_OF_MEMORY; } - CompactObj& co = add_res.first->second; + CompactObj& co = it->second; - if (!add_res.second) { + if (!new_key) { // for non-overwrite case it must be set. if (!overwrite && co.ObjType() != OBJ_SET) return OpStatus::WRONG_TYPE; // Update stats and trigger any handle the old value if needed. - db_slice.PreUpdate(op_args.db_ind, add_res.first); + db_slice.PreUpdate(op_args.db_ind, it); } - if (add_res.second || overwrite) { + if (new_key || overwrite) { // does not store the values, merely sets the encoding. // TODO: why not store the values as well? InitSet(vals, &co); @@ -379,7 +381,7 @@ OpResult OpAdd(const OpArgs& op_args, std::string_view key, ArgSlice v } } - db_slice.PostUpdate(op_args.db_ind, add_res.first); + db_slice.PostUpdate(op_args.db_ind, it, !new_key); return res; } diff --git a/src/server/string_family.cc b/src/server/string_family.cc index da340fe..00df09e 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -100,7 +100,7 @@ OpResult OpSetRange(const OpArgs& op_args, string_view key, size_t sta memcpy(s.data() + start, value.data(), value.size()); it->second.SetString(s); - db_slice.PostUpdate(op_args.db_ind, it); + db_slice.PostUpdate(op_args.db_ind, it, !added); RecordJournal(op_args, it->first, it->second); return it->second.Size(); @@ -138,15 +138,33 @@ OpResult OpGetRange(const OpArgs& op_args, string_view key, int32_t star return string(slice.substr(start, end - start + 1)); }; +size_t ExtendExisting(const OpArgs& op_args, PrimeIterator it, string_view val, bool prepend) { + string tmp, new_val; + auto* shard = op_args.shard; + string_view slice = GetSlice(shard, it->second, &tmp); + if (prepend) + new_val = absl::StrCat(val, slice); + else + new_val = absl::StrCat(slice, val); + + auto& db_slice = shard->db_slice(); + db_slice.PreUpdate(op_args.db_ind, it); + it->second.SetString(new_val); + db_slice.PostUpdate(op_args.db_ind, it, true); + RecordJournal(op_args, it->first, it->second); + + return new_val.size(); +} + // Returns the length of the extended string. if prepend is false - appends the val. -OpResult ExtendOrSet(const OpArgs& op_args, std::string_view key, std::string_view val, +OpResult ExtendOrSet(const OpArgs& op_args, string_view key, string_view val, bool prepend) { auto* shard = op_args.shard; auto& db_slice = shard->db_slice(); auto [it, inserted] = db_slice.AddOrFind(op_args.db_ind, key); if (inserted) { it->second.SetString(val); - db_slice.PostUpdate(op_args.db_ind, it); + db_slice.PostUpdate(op_args.db_ind, it, false); RecordJournal(op_args, it->first, it->second); return val.size(); @@ -155,19 +173,7 @@ OpResult ExtendOrSet(const OpArgs& op_args, std::string_view key, std: if (it->second.ObjType() != OBJ_STRING) return OpStatus::WRONG_TYPE; - string tmp, new_val; - string_view slice = GetSlice(op_args.shard, it->second, &tmp); - if (prepend) - new_val = absl::StrCat(val, slice); - else - new_val = absl::StrCat(slice, val); - - db_slice.PreUpdate(op_args.db_ind, it); - it->second.SetString(new_val); - db_slice.PostUpdate(op_args.db_ind, it); - RecordJournal(op_args, it->first, it->second); - - return new_val.size(); + return ExtendExisting(op_args, it, val, prepend); } OpResult ExtendOrSkip(const OpArgs& op_args, std::string_view key, std::string_view val, @@ -178,20 +184,7 @@ OpResult ExtendOrSkip(const OpArgs& op_args, std::string_view key, std::st return false; } - CompactObj& cobj = (*it_res)->second; - - string tmp, new_val; - string_view slice = GetSlice(op_args.shard, cobj, &tmp); - if (prepend) - new_val = absl::StrCat(val, slice); - else - new_val = absl::StrCat(slice, val); - - db_slice.PreUpdate(op_args.db_ind, *it_res); - cobj.SetString(new_val); - db_slice.PostUpdate(op_args.db_ind, *it_res); - - return new_val.size(); + return ExtendExisting(op_args, *it_res, val, prepend); } OpResult OpGet(const OpArgs& op_args, string_view key) { @@ -213,7 +206,7 @@ OpResult OpIncrFloat(const OpArgs& op_args, std::string_view key, double if (inserted) { char* str = RedisReplyBuilder::FormatDouble(val, buf, sizeof(buf)); it->second.SetString(str); - db_slice.PostUpdate(op_args.db_ind, it); + db_slice.PostUpdate(op_args.db_ind, it, false); RecordJournal(op_args, it->first, it->second); return val; @@ -245,7 +238,7 @@ OpResult OpIncrFloat(const OpArgs& op_args, std::string_view key, double db_slice.PreUpdate(op_args.db_ind, it); it->second.SetString(str); - db_slice.PostUpdate(op_args.db_ind, it); + db_slice.PostUpdate(op_args.db_ind, it, true); RecordJournal(op_args, it->first, it->second); return base; @@ -360,7 +353,7 @@ OpStatus SetCmd::Set(const SetParams& params, string_view key, string_view value PrimeValue tvalue{value}; tvalue.SetFlag(params.memcache_flags != 0); it->second = std::move(tvalue); - db_slice.PostUpdate(params.db_index, it); + db_slice.PostUpdate(params.db_index, it, false); if (params.expire_after_ms) { db_slice.UpdateExpire(params.db_index, it, params.expire_after_ms + db_slice.Now()); diff --git a/src/server/table.cc b/src/server/table.cc index 9ead685..76f9802 100644 --- a/src/server/table.cc +++ b/src/server/table.cc @@ -15,11 +15,12 @@ unsigned kInitSegmentLog = 3; DbTableStats& DbTableStats::operator+=(const DbTableStats& o) { constexpr size_t kDbSz = sizeof(DbTableStats); - static_assert(kDbSz == 56); + static_assert(kDbSz == 64); ADD(inline_keys); ADD(obj_memory_usage); ADD(strval_memory_usage); + ADD(update_value_amount); ADD(listpack_blob_cnt); ADD(listpack_bytes); ADD(external_entries); diff --git a/src/server/table.h b/src/server/table.h index 552f9c2..7960e5c 100644 --- a/src/server/table.h +++ b/src/server/table.h @@ -42,6 +42,9 @@ struct DbTableStats { // Applies for any non-inline objects. size_t obj_memory_usage = 0; size_t strval_memory_usage = 0; + + // how much we we increased or decreased the existing entries. + ssize_t update_value_amount = 0; size_t listpack_blob_cnt = 0; size_t listpack_bytes = 0; size_t external_entries = 0;