Skip to content

Commit 27638cc

Browse files
authored
fix(core): Account for size change on json object defragment (#6053)
core: Record change in size after defrag Signed-off-by: Abhijat Malviya <abhijat@dragonflydb.io>
1 parent 0352446 commit 27638cc

File tree

3 files changed

+37
-3
lines changed

3 files changed

+37
-3
lines changed

src/core/compact_object.cc

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1610,11 +1610,25 @@ MemoryResource* CompactObj::memory_resource() {
16101610
}
16111611

16121612
bool CompactObj::JsonConsT::DefragIfNeeded(PageUsage* page_usage) {
1613-
if (JsonType* old = json_ptr; ShouldDefragment(page_usage)) {
1613+
if (ShouldDefragment(page_usage)) {
1614+
const MiMemoryResource* mr = static_cast<MiMemoryResource*>(memory_resource());
1615+
1616+
const int64_t before = static_cast<int64_t>(mr->used());
1617+
DCHECK_GE(before, 0) << "Memory usage is more than int64_t max value";
1618+
1619+
JsonType* old = json_ptr;
16141620
json_ptr = AllocateMR<JsonType>(DeepCopyJSON(old));
16151621
DeleteMR<JsonType>(old);
1622+
1623+
const int64_t after = static_cast<int64_t>(mr->used());
1624+
DCHECK_GE(after, 0) << "Memory usage is more than int64_t max value";
1625+
1626+
if (const int64_t delta = after - before; delta != 0) {
1627+
bytes_used = UpdateSize(bytes_used, delta);
1628+
}
16161629
return true;
16171630
}
1631+
16181632
return false;
16191633
}
16201634

src/core/page_usage_stats_test.cc

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,17 +194,31 @@ TEST_F(PageUsageStatsTest, JSONCons) {
194194
// still fail. This is because freeing the compact object code path takes the wrong branch based
195195
// on encoding. The flat encoding was tested manually adjusting this same test with changed
196196
// encoding.
197-
std::string_view data{R"#({"data": "some", "count": 1, "checked": false})#"};
197+
std::string data = R"({"contents":[)";
198+
for (size_t i = 0; i < 1000; ++i) {
199+
const auto si = std::to_string(i);
200+
data += R"({"id":)" + si + R"(,"class":"v___)" + si + R"("})";
201+
if (i < 999) {
202+
data += ",";
203+
}
204+
}
205+
data += R"(], "data": "some", "count": 1, "checked": false})";
206+
207+
auto* mr = static_cast<MiMemoryResource*>(CompactObj::memory_resource());
208+
size_t before = mr->used();
198209

199210
auto parsed = ParseJsonUsingShardHeap(data);
200211
EXPECT_TRUE(parsed.has_value());
201212

202213
c_obj_.SetJson(std::move(parsed.value()));
214+
c_obj_.SetJsonSize(mr->used() - before);
215+
EXPECT_GT(c_obj_.MallocUsed(), 0);
203216

204217
PageUsage p{CollectPageStats::YES, 0.1};
205218
p.SetForceReallocate(true);
206219

207220
c_obj_.DefragIfNeeded(&p);
221+
EXPECT_GT(c_obj_.MallocUsed(), 0);
208222

209223
const auto stats = p.CollectedStats();
210224
EXPECT_GT(stats.pages_scanned, 0);

src/server/engine_shard.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,14 +351,20 @@ std::optional<CollectedPageStats> EngineShard::DoDefrag(CollectPageStats collect
351351
uint64_t attempts = 0;
352352

353353
PageUsage page_usage{collect_page_stats, threshold};
354+
DbTable* db_table = slice.GetDBTable(defrag_state_.dbid);
354355
do {
355356
cur = prime_table->Traverse(cur, [&](PrimeIterator it) {
356357
// for each value check whether we should move it because it
357358
// seats on underutilized page of memory, and if so, do it.
358-
bool did = it->second.DefragIfNeeded(&page_usage);
359+
const ssize_t original_size = it->second.MallocUsed();
360+
const bool did = it->second.DefragIfNeeded(&page_usage);
359361
attempts++;
360362
if (did) {
361363
reallocations++;
364+
if (const ssize_t delta = it->second.MallocUsed() - original_size;
365+
delta != 0 && db_table != nullptr) {
366+
db_table->stats.AddTypeMemoryUsage(it->second.ObjType(), delta);
367+
}
362368
}
363369
});
364370
traverses_count++;

0 commit comments

Comments
 (0)