SERVER-65473 stop underflow assert in global memory tracker

2 files changed, 29 insertions, 26 deletions

diff --git a/src/mongo/db/pipeline/memory_usage_tracker.h b/src/mongo/db/pipeline/memory_usage_tracker.h
index 275183081ef..9847b41e3cb 100644
--- a/src/mongo/db/pipeline/memory_usage_tracker.h
+++ b/src/mongo/db/pipeline/memory_usage_tracker.h
@@ -53,15 +53,8 @@ public:
 
             // TODO SERVER-61281: this is a temporary measure in tackling the problem in this
             // ticket. It prevents the underflow from happening but doesn't address the cause
-            // which is inaccurate tracking.
-            // Once inaccurate tracking is resolved, the underflow assertion below could be
-            // restored.
-            //      tassert(5578603,
-            //              str::stream() << "Underflow on memory tracking, attempting to add " <<
-            //              diff
-            //                            << " but only " << _currentMemoryBytes << " available",
-            //              diff >= 0 || _currentMemoryBytes >= std::abs(diff));
-            //      set(_currentMemoryBytes + diff);
+            // which is inaccurate tracking. Once inaccurate tracking is resolved, the underflow
+            // assertion could be restored. See SERVER-62856 and SERVER-65473 for details.
 
             set(std::max(_currentMemoryBytes + diff, 0LL));
         }
@@ -154,11 +147,13 @@ public:
      * Updates total memory usage.
      */
     void update(long long diff) {
-        tassert(5578602,
-                str::stream() << "Underflow on memory tracking, attempting to add " << diff
-                              << " but only " << _memoryUsageBytes << " available",
-                diff >= 0 || (int)_memoryUsageBytes >= -1 * diff);
-        set(_memoryUsageBytes + diff);
+
+        // TODO SERVER-61281: this is a temporary measure in tackling the problem in this
+        // ticket. It prevents the underflow from happening but doesn't address the cause
+        // which is inaccurate tracking. Once inaccurate tracking is resolved, the underflow
+        // assertion could be restored. See SERVER-62856 and SERVER-65473 for details.
+
+        set(std::max(_memoryUsageBytes + diff, 0LL));
     }
 
     auto currentMemoryBytes() const {
diff --git a/src/mongo/db/pipeline/memory_usage_tracker_test.cpp b/src/mongo/db/pipeline/memory_usage_tracker_test.cpp
index cc354e7fd69..81ca06a1381 100644
--- a/src/mongo/db/pipeline/memory_usage_tracker_test.cpp
+++ b/src/mongo/db/pipeline/memory_usage_tracker_test.cpp
@@ -99,27 +99,35 @@ TEST_F(MemoryUsageTrackerTest, UpdateUsageUpdatesGlobal) {
     ASSERT_EQ(_tracker.maxMemoryBytes(), 150LL);
 }
 
-DEATH_TEST_F(MemoryUsageTrackerTest,
-             UpdateGlobalToNegativeIsDisallowed,
-             "Underflow on memory tracking") {
-    _tracker.set(50LL);
+// TODO SERVER-61281: Need to add back the DEATH_TEST_F removed in SERVER-65473, once
+// MemoryUsageTracker.update() no longer underflows.
+
+TEST_F(MemoryUsageTrackerTest, UpdateFunctionUsageToNegativeIsDisallowed) {
+    _funcTracker.set(50LL);
+    ASSERT_EQ(_funcTracker.currentMemoryBytes(), 50LL);
+    ASSERT_EQ(_funcTracker.maxMemoryBytes(), 50LL);
     ASSERT_EQ(_tracker.currentMemoryBytes(), 50LL);
     ASSERT_EQ(_tracker.maxMemoryBytes(), 50LL);
 
-    _tracker.update(-100);
+    // TODO SERVER-61281: This should throw an assertion once accurate tracking is implemented and
+    // no underflow should happen. That assertion would make sure "Underflow in memory tracking"
+    // is reported.
+
+    _funcTracker.update(-100);
+    ASSERT_EQ(_funcTracker.currentMemoryBytes(), 0LL);
+    ASSERT_EQ(_tracker.currentMemoryBytes(), 0LL);
 }
 
-TEST_F(MemoryUsageTrackerTest, UpdateFunctionUsageToNegativeIsDisallowed) {
-    _funcTracker.set(50LL);
+TEST_F(MemoryUsageTrackerTest, UpdateMemUsageToNegativeIsDisallowed) {
+    _tracker.set(50LL);
     ASSERT_EQ(_tracker.currentMemoryBytes(), 50LL);
     ASSERT_EQ(_tracker.maxMemoryBytes(), 50LL);
 
-    // TODO SERVER-61281: Temporarily disable the assert (and associated test) in
-    // PerFunctionMemoryTracker.update() to prevent inaccurate tracking to cause underflow errors
-    // Once accurate tracking is implemented and no underflow should happen, this negative test
-    // could be restored to verify that "Underflow on memory tracking" is reported.
+    // TODO SERVER-61281: This should throw an assertion once accurate tracking is implemented and
+    // no underflow should happen. That assertion would make sure "Underflow in memory tracking"
+    // is reported.
 
-    _funcTracker.update(-100);
+    _tracker.update(-100);
     ASSERT_EQ(_tracker.currentMemoryBytes(), 0LL);
 }