From ae9c8eee4eb005f404dbd6374b5e1a0684fe3d0a Mon Sep 17 00:00:00 2001 From: Jialiang Tan Date: Tue, 14 May 2024 20:57:13 -0700 Subject: [PATCH] [native] Remove spill stats reporting from presto native --- .../presto_cpp/main/PeriodicTaskManager.cpp | 49 +------------------ .../presto_cpp/main/PeriodicTaskManager.h | 19 ------- .../presto_cpp/main/common/Counters.cpp | 22 --------- .../presto_cpp/main/common/Counters.h | 44 ----------------- presto-native-execution/velox | 2 +- 5 files changed, 2 insertions(+), 134 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/PeriodicTaskManager.cpp b/presto-native-execution/presto_cpp/main/PeriodicTaskManager.cpp index 4e39e248f2d0..861cf7425a38 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicTaskManager.cpp +++ b/presto-native-execution/presto_cpp/main/PeriodicTaskManager.cpp @@ -84,7 +84,6 @@ static constexpr size_t kTaskPeriodCleanOldTasks{60'000'000}; // 60 seconds. static constexpr size_t kConnectorPeriodGlobalCounters{ 60'000'000}; // 60 seconds. static constexpr size_t kOsPeriodGlobalCounters{2'000'000}; // 2 seconds -static constexpr size_t kSpillStatsUpdateIntervalUs{60'000'000}; // 60 seconds // Every 1 minute we print endpoint latency counters. static constexpr size_t kHttpEndpointLatencyPeriodGlobalCounters{ 60'000'000}; // 60 seconds. @@ -114,6 +113,7 @@ void PeriodicTaskManager::start() { opts.arbitrator = arbitrator_->kind() == "NOOP" ? nullptr : arbitrator_; opts.allocator = memoryAllocator_; opts.cache = asyncDataCache_; + opts.spillMemoryPool = velox::memory::spillMemoryPool(); velox::startPeriodicStatsReporter(opts); // If executors are null, don't bother starting this task. @@ -134,8 +134,6 @@ void PeriodicTaskManager::start() { addOperatingSystemStatsUpdateTask(); - addSpillStatsUpdateTask(); - if (SystemConfig::instance()->enableHttpEndpointLatencyFilter()) { addHttpEndpointLatencyStatsTask(); } @@ -393,51 +391,6 @@ void PeriodicTaskManager::addOperatingSystemStatsUpdateTask() { "os_counters"); } -void PeriodicTaskManager::addSpillStatsUpdateTask() { - addTask( - [this]() { updateSpillStatsTask(); }, - kSpillStatsUpdateIntervalUs, - "spill_stats"); -} - -void PeriodicTaskManager::updateSpillStatsTask() { - const auto updatedSpillStats = velox::common::globalSpillStats(); - VELOX_CHECK_GE(updatedSpillStats, lastSpillStats_); - const auto deltaSpillStats = updatedSpillStats - lastSpillStats_; - REPORT_IF_NOT_ZERO(kCounterSpillRuns, deltaSpillStats.spillRuns); - REPORT_IF_NOT_ZERO(kCounterSpilledFiles, deltaSpillStats.spilledFiles); - REPORT_IF_NOT_ZERO(kCounterSpilledRows, deltaSpillStats.spilledRows); - REPORT_IF_NOT_ZERO(kCounterSpilledBytes, deltaSpillStats.spilledBytes); - REPORT_IF_NOT_ZERO(kCounterSpillFillTimeUs, deltaSpillStats.spillFillTimeUs); - REPORT_IF_NOT_ZERO(kCounterSpillSortTimeUs, deltaSpillStats.spillSortTimeUs); - REPORT_IF_NOT_ZERO( - kCounterSpillSerializationTimeUs, - deltaSpillStats.spillSerializationTimeUs); - REPORT_IF_NOT_ZERO(kCounterSpillWrites, deltaSpillStats.spillWrites); - REPORT_IF_NOT_ZERO( - kCounterSpillFlushTimeUs, deltaSpillStats.spillFlushTimeUs); - REPORT_IF_NOT_ZERO( - kCounterSpillWriteTimeUs, deltaSpillStats.spillWriteTimeUs); - REPORT_IF_NOT_ZERO( - kCounterSpillMaxLevelExceeded, - deltaSpillStats.spillMaxLevelExceededCount); - - if (!deltaSpillStats.empty()) { - LOG(INFO) << "Updated spill stats: " << updatedSpillStats.toString(); - LOG(INFO) << "Spill stats change:" << deltaSpillStats.toString(); - } - - const auto spillMemoryStats = velox::memory::spillMemoryPool()->stats(); - LOG(INFO) << "Spill memory usage: current[" - << velox::succinctBytes(spillMemoryStats.currentBytes) << "] peak[" - << velox::succinctBytes(spillMemoryStats.peakBytes) << "]"; - RECORD_METRIC_VALUE(kCounterSpillMemoryBytes, spillMemoryStats.currentBytes); - RECORD_HISTOGRAM_METRIC_VALUE( - kCounterSpillPeakMemoryBytes, spillMemoryStats.peakBytes); - - lastSpillStats_ = updatedSpillStats; -} - void PeriodicTaskManager::printHttpEndpointLatencyStats() { const auto latencyMetrics = http::filters::HttpEndpointLatencyFilter::retrieveLatencies(); diff --git a/presto-native-execution/presto_cpp/main/PeriodicTaskManager.h b/presto-native-execution/presto_cpp/main/PeriodicTaskManager.h index 0098eecdd9f2..05c81f58f862 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicTaskManager.h +++ b/presto-native-execution/presto_cpp/main/PeriodicTaskManager.h @@ -16,7 +16,6 @@ #include #include #include "velox/common/memory/Memory.h" -#include "velox/exec/Spill.h" #include "velox/exec/Task.h" namespace folly { @@ -112,9 +111,6 @@ class PeriodicTaskManager { void addOperatingSystemStatsUpdateTask(); void updateOperatingSystemStats(); - void addSpillStatsUpdateTask(); - void updateSpillStatsTask(); - // Adds task that periodically prints http endpoint latency metrics. void addHttpEndpointLatencyStatsTask(); void printHttpEndpointLatencyStats(); @@ -135,19 +131,6 @@ class PeriodicTaskManager { std::shared_ptr>& connectors_; PrestoServer* const server_; - // Cache related stats - int64_t lastMemoryCacheHits_{0}; - int64_t lastMemoryCacheHitsBytes_{0}; - int64_t lastMemoryCacheInserts_{0}; - int64_t lastMemoryCacheEvictions_{0}; - int64_t lastMemoryCacheEvictionChecks_{0}; - int64_t lastMemoryCacheStalls_{0}; - int64_t lastMemoryCacheAllocClocks_{0}; - int64_t lastMemoryCacheAgedOuts_{0}; - int64_t lastSsdCacheCheckpointsWritten_{0}; - int64_t lastSsdCacheCheckpointsRead_{0}; - int64_t lastSsdCacheRegionsEvicted_{0}; - // Operating system related stats. int64_t lastUserCpuTimeUs_{0}; int64_t lastSystemCpuTimeUs_{0}; @@ -155,8 +138,6 @@ class PeriodicTaskManager { int64_t lastHardPageFaults_{0}; int64_t lastVoluntaryContextSwitches_{0}; int64_t lastForcedContextSwitches_{0}; - // Renabled this after update velox. - velox::common::SpillStats lastSpillStats_; // NOTE: declare last since the threads access other members of `this`. folly::FunctionScheduler oneTimeRunner_; diff --git a/presto-native-execution/presto_cpp/main/common/Counters.cpp b/presto-native-execution/presto_cpp/main/common/Counters.cpp index 59c864a90089..e6d382aac016 100644 --- a/presto-native-execution/presto_cpp/main/common/Counters.cpp +++ b/presto-native-execution/presto_cpp/main/common/Counters.cpp @@ -110,28 +110,6 @@ void registerPrestoMetrics() { 62l * 1024 * 1024 * 1024, // max bucket value: 62GB 100); - /// ================== Disk Spilling Counters ================= - - DEFINE_METRIC(kCounterSpillRuns, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpilledFiles, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpilledRows, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpilledBytes, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillFillTimeUs, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillSortTimeUs, facebook::velox::StatType::SUM); - DEFINE_METRIC( - kCounterSpillSerializationTimeUs, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillWrites, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillFlushTimeUs, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillWriteTimeUs, facebook::velox::StatType::SUM); - DEFINE_METRIC(kCounterSpillMemoryBytes, facebook::velox::StatType::AVG); - DEFINE_HISTOGRAM_METRIC( - kCounterSpillPeakMemoryBytes, - 1l * 512 * 1024 * 1024, - 0, - 20l * 1024 * 1024 * 1024, // max bucket value: 20GB - 100); - DEFINE_METRIC(kCounterSpillMaxLevelExceeded, facebook::velox::StatType::SUM); - // NOTE: Metrics type exporting for file handle cache counters are in // PeriodicTaskManager because they have dynamic names. The following counters // have their type exported there: diff --git a/presto-native-execution/presto_cpp/main/common/Counters.h b/presto-native-execution/presto_cpp/main/common/Counters.h index 50509ce88928..c4cd6a78fb02 100644 --- a/presto-native-execution/presto_cpp/main/common/Counters.h +++ b/presto-native-execution/presto_cpp/main/common/Counters.h @@ -138,50 +138,6 @@ constexpr folly::StringPiece kCounterOsNumVoluntaryContextSwitches{ constexpr folly::StringPiece kCounterOsNumForcedContextSwitches{ "presto_cpp.os_num_forced_context_switches"}; -/// ================== Disk Spilling Counters ================= - -/// The number of times that spilling runs on a velox operator. -constexpr folly::StringPiece kCounterSpillRuns{"presto_cpp.spill_run_count"}; -/// The number of spilled files. -constexpr folly::StringPiece kCounterSpilledFiles{ - "presto_cpp.spilled_file_count"}; -/// The number of spilled rows. -constexpr folly::StringPiece kCounterSpilledRows{ - "presto_cpp.spilled_row_count"}; -/// The number of bytes spilled to disks. -/// -/// NOTE: if compression is enabled, this counts the compressed bytes. -constexpr folly::StringPiece kCounterSpilledBytes{"presto_cpp.spilled_bytes"}; -/// The time spent on filling rows for spilling. -constexpr folly::StringPiece kCounterSpillFillTimeUs{ - "presto_cpp.spill_fill_time_us"}; -/// The time spent on sorting rows for spilling. -constexpr folly::StringPiece kCounterSpillSortTimeUs{ - "presto_cpp.spill_sort_time_us"}; -/// The time spent on serializing rows for spilling. -constexpr folly::StringPiece kCounterSpillSerializationTimeUs{ - "presto_cpp.spill_serialization_time_us"}; -/// The number of disk writes to spill rows. -constexpr folly::StringPiece kCounterSpillWrites{ - "presto_cpp.spill_write_count"}; -/// The time spent on copy out serialized rows for disk write. If compression -/// is enabled, this includes the compression time. -constexpr folly::StringPiece kCounterSpillFlushTimeUs{ - "presto_cpp.spill_flush_time_us"}; -/// The time spent on writing spilled rows to disk. -constexpr folly::StringPiece kCounterSpillWriteTimeUs{ - "presto_cpp.spill_write_time_us"}; -/// The number of times that a spillable operator exceeds the max spill level -/// limit that can't spill. -constexpr folly::StringPiece kCounterSpillMaxLevelExceeded{ - "presto_cpp.spill_exceeded_max_level_count"}; -/// The current spilling memory usage in bytes. -constexpr folly::StringPiece kCounterSpillMemoryBytes{ - "presto_cpp.spill_memory_bytes"}; -/// The peak spilling memory usage in bytes. -constexpr folly::StringPiece kCounterSpillPeakMemoryBytes{ - "presto_cpp.spill_peak_memory_bytes"}; - /// ================== HiveConnector Counters ================== /// Format template strings use 'constexpr std::string_view' to be 'fmt::format' /// compatible. diff --git a/presto-native-execution/velox b/presto-native-execution/velox index 2c98308b4563..54db95234190 160000 --- a/presto-native-execution/velox +++ b/presto-native-execution/velox @@ -1 +1 @@ -Subproject commit 2c98308b4563d0c58ab016708b835bb7fce4a9ce +Subproject commit 54db95234190a14723cb9792ae48a77823368029