diff --git a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp index e10eb2ae15ebc..6f5d9cc550334 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp +++ b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp @@ -211,6 +211,8 @@ void PeriodicMemoryChecker::pushbackMemory() { kCounterMemoryPushbackLatencyMs, latencyUs / 1000); const auto actualFreedBytes = std::max( 0, static_cast(currentMemBytes) - systemUsedMemoryBytes()); + RECORD_HISTOGRAM_METRIC_VALUE( + kCounterMemoryPushbackExpectedReductionBytes, freedBytes); RECORD_HISTOGRAM_METRIC_VALUE( kCounterMemoryPushbackReductionBytes, actualFreedBytes); LOG(INFO) << "Memory pushback shrunk " << velox::succinctBytes(freedBytes) diff --git a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h index 239637b020691..d16fecdcf92e2 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h +++ b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h @@ -87,7 +87,7 @@ class PeriodicMemoryChecker { /// Callback function that is invoked by 'PeriodicMemoryChecker' periodically. /// Light operations such as stats reporting can be done in this call back. - virtual void periodicCb() const = 0; + virtual void periodicCb() = 0; /// Callback function that performs a heap dump. Returns true if dump is /// successful. diff --git a/presto-native-execution/presto_cpp/main/common/Counters.cpp b/presto-native-execution/presto_cpp/main/common/Counters.cpp index a395596370120..0a1b5afc768c6 100644 --- a/presto-native-execution/presto_cpp/main/common/Counters.cpp +++ b/presto-native-execution/presto_cpp/main/common/Counters.cpp @@ -103,7 +103,16 @@ void registerPrestoMetrics() { DEFINE_HISTOGRAM_METRIC( kCounterMemoryPushbackLatencyMs, 10'000, 0, 100'000, 50, 90, 99, 100); DEFINE_HISTOGRAM_METRIC( - kCounterMemoryPushbackLatencyMs, + kCounterMemoryPushbackReductionBytes, + 100l * 1024 * 1024, // 100MB + 0, + 15l * 1024 * 1024 * 1024, // 15GB + 50, + 90, + 99, + 100); + DEFINE_HISTOGRAM_METRIC( + kCounterMemoryPushbackExpectedReductionBytes, 100l * 1024 * 1024, // 100MB 0, 15l * 1024 * 1024 * 1024, // 15GB diff --git a/presto-native-execution/presto_cpp/main/common/Counters.h b/presto-native-execution/presto_cpp/main/common/Counters.h index 6369671c05440..8f732d7663485 100644 --- a/presto-native-execution/presto_cpp/main/common/Counters.h +++ b/presto-native-execution/presto_cpp/main/common/Counters.h @@ -166,9 +166,16 @@ constexpr folly::StringPiece kCounterMemoryPushbackCount{ /// reports P50, P90, P99, and P100. constexpr folly::StringPiece kCounterMemoryPushbackLatencyMs{ "presto_cpp.memory_pushback_latency_ms"}; -/// Distribution of reduction in memory usage achieved by each memory pushback -/// attempt. This is to gauge its effectiveness. In range of [0, 15GB] with 150 -/// buckets and reports P50, P90, P99, and P100. +/// Distribution of actual reduction in memory usage achieved by each memory +/// pushback attempt. This is to gauge its effectiveness. In range of [0, 15GB] +/// with 150 buckets and reports P50, P90, P99, and P100. constexpr folly::StringPiece kCounterMemoryPushbackReductionBytes{ "presto_cpp.memory_pushback_reduction_bytes"}; +/// Distribution of expected reduction in memory usage achieved by each memory +/// pushback attempt. This is to gauge its effectiveness. In range of [0, 15GB] +/// with 150 buckets and reports P50, P90, P99, and P100. The expected reduction +/// can be different as other threads might have allocated memory in the +/// meantime. +constexpr folly::StringPiece kCounterMemoryPushbackExpectedReductionBytes{ + "presto_cpp.memory_pushback_expected_reduction_bytes"}; } // namespace facebook::presto diff --git a/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp b/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp index 890cdc5704101..3ac1b8237a9b0 100644 --- a/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp +++ b/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp @@ -54,7 +54,7 @@ class PeriodicMemoryCheckerTest : public testing::Test { return mallocBytes_; } - void periodicCb() const override { + void periodicCb() override { if (periodicCb_) { periodicCb_(); }