fix: correct per-type memory accounting when object type changes in AutoUpdater (#7142)

AutoUpdater::Run() applied the full delta (new_size - orig_size) to the
current object type. When AddOrUpdate replaces a value with a different
type (e.g., SORT STORE overwrites a SET with a LIST), the original bytes
were tracked under the old type but the delta was subtracted from the new
type's counter — which never had those bytes — causing an underflow crash.

- Add orig_obj_type to AutoUpdater::Fields, captured at construction time
- In Run(): when type changed, separately subtract orig_size from old type
  and add current_size to new type, instead of applying a signed delta to
  the new type
- In ReduceHeapUsage(): use orig_obj_type for correct accounting, then sync
  it to current type so subsequent Run() sees consistent state
- In AddTypeMemoryUsage(): crash with FATAL in debug builds on underflow
  (was ERROR) and log the per-type counter value instead of obj_memory_usage

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

Author: romange

src/server/db_slice.cc

@@ -473,9 +473,10 @@ DbSlice::AutoUpdater::~AutoUpdater() {
 }
 
 void DbSlice::AutoUpdater::ReduceHeapUsage() {
-  AccountObjectMemory(fields_.key, fields_.it->second.ObjType(), -fields_.orig_value_heap_size,
+  AccountObjectMemory(fields_.key, fields_.orig_obj_type, -fields_.orig_value_heap_size,
                       fields_.db_slice->GetDBTable(fields_.db_ind));
-  fields_.orig_value_heap_size = 0;  // Reset to avoid double accounting.
+  fields_.orig_value_heap_size = 0;                      // Reset to avoid double accounting.
+  fields_.orig_obj_type = fields_.it->second.ObjType();  // Sync type after accounting.
 }
 
 void DbSlice::AutoUpdater::Run() {
@@ -490,10 +491,22 @@ void DbSlice::AutoUpdater::Run() {
 
   CHECK_NE(fields_.db_slice, nullptr);
 
-  ssize_t delta = static_cast<int64_t>(fields_.it->second.MallocUsed()) -
-                  static_cast<int64_t>(fields_.orig_value_heap_size);
-  AccountObjectMemory(fields_.key, fields_.it->second.ObjType(), delta,
-                      fields_.db_slice->GetDBTable(fields_.db_ind));
+  CompactObjType current_type = fields_.it->second.ObjType();
+  int64_t current_size = static_cast<int64_t>(fields_.it->second.MallocUsed());
+  DbTable* table = fields_.db_slice->GetDBTable(fields_.db_ind);
+
+  if (current_type != fields_.orig_obj_type) {
+    // Type changed: remove old size from old type, add new size to new type separately.
+    // Applying (current_size - orig_size) to the new type would incorrectly subtract
+    // from a counter that never had the original bytes added to it.
+    AccountObjectMemory(fields_.key, fields_.orig_obj_type,
+                        -static_cast<int64_t>(fields_.orig_value_heap_size), table);
+    AccountObjectMemory(fields_.key, current_type, current_size, table);
+  } else {
+    ssize_t delta = current_size - static_cast<int64_t>(fields_.orig_value_heap_size);
+    AccountObjectMemory(fields_.key, current_type, delta, table);
+  }
+
   fields_.db_slice->PostUpdate(fields_.db_ind, fields_.key);
   Cancel();  // Reset to not run again
 }
@@ -508,7 +521,8 @@ DbSlice::AutoUpdater::AutoUpdater(DbIndex db_ind, std::string_view key, const It
               .db_ind = db_ind,
               .it = it,
               .key = key,
-              .orig_value_heap_size = it->second.MallocUsed()} {
+              .orig_value_heap_size = it->second.MallocUsed(),
+              .orig_obj_type = it->second.ObjType()} {
   DCHECK(IsValid(it));
 }
 

src/server/db_slice.h

@@ -175,6 +175,7 @@ class DbSlice {
 
       // The following fields are calculated at init time
       size_t orig_value_heap_size = 0;
+      CompactObjType orig_obj_type = 0;
     };
 
     AutoUpdater(DbIndex db_ind, std::string_view key, const Iterator& it, DbSlice* db_slice);

src/server/table.cc

@@ -26,8 +26,13 @@ void DbTableStats::AddTypeMemoryUsage(unsigned type, int64_t delta) {
   DCHECK_GE(obj_memory_usage, memory_usage_by_type[type]);
 
   if (delta < 0 && memory_usage_by_type[type] < size_t(-delta)) {
-    LOG_EVERY_T(ERROR, 1) << "Encountered underflow memory usage when aggregating per-type memory: "
-                          << obj_memory_usage << " + " << delta << ", type: " << type;
+#ifdef NDEBUG
+    LOG_EVERY_T(ERROR, 1)
+#else
+    LOG_EVERY_T(FATAL, 1)
+#endif
+        << "Encountered underflow memory usage when aggregating per-type memory: "
+        << memory_usage_by_type[type] << " + " << delta << ", type: " << type;
 
     // Truncate delta to avoid underflow, but keep the memory usage consistent with the sum of
     // per-type usage.