From ec60c1377a0a2ff4b2f0ebce6d831b7eca71fddf Mon Sep 17 00:00:00 2001 From: Zac Wen Date: Wed, 20 Nov 2024 11:10:55 -0800 Subject: [PATCH] [native] Add config for cpp cache default behavior In Presto Java, cache is disabled by default and only enabled when the config `node_preference` is set to `SOFT_AFFINITY`, while in CPP, cache is enabled by default and only disabled when the config node_preference is set to "NO_PREFERENCE". This change adds a new config to support the alignment of the default cache behavior. --- .../src/main/sphinx/presto_cpp/features.rst | 11 +++++++++ .../presto_cpp/main/QueryContextManager.cpp | 23 ++++++++++++++----- .../presto_cpp/main/common/Configs.cpp | 5 ++++ .../presto_cpp/main/common/Configs.h | 8 +++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/presto-docs/src/main/sphinx/presto_cpp/features.rst b/presto-docs/src/main/sphinx/presto_cpp/features.rst index 9d3ed7c433eb9..6dd747f3a4d02 100644 --- a/presto-docs/src/main/sphinx/presto_cpp/features.rst +++ b/presto-docs/src/main/sphinx/presto_cpp/features.rst @@ -184,6 +184,17 @@ disabled if ``connector.num-io-threads-hw-multiplier`` is set to zero. Whether async data cache is enabled. +``query-data-cache-enabled-default`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* **Type** ``bool`` +* **Default value:** ``true`` + +If ``true``, SSD cache is enabled by default and is disabled only if +``node_selection_strategy`` is present and set to ``NO_PREFERENCE``. +Otherwise, SSD cache is disabled by default and is enabled if +``node_selection_strategy`` is present and set to ``SOFT_AFFINITY``. + ``async-cache-ssd-gb`` ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp index a5714fb7897bd..767231a3bfa03 100644 --- a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp +++ b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp @@ -98,15 +98,26 @@ void updateVeloxConnectorConfigs( std::unordered_map< std::string, std::unordered_map>& connectorConfigStrings) { + const auto& systemConfig = SystemConfig::instance(); for (auto& entry : connectorConfigStrings) { auto& connectorConfig = entry.second; - - // Do not retain cache if `node_selection_strategy` is explicitly set to - // `NO_PREFERENCE`. + // If queryDataCacheEnabledDefault is true, when `node_selection_strategy` + // is + // not set retain cache + // SOFT_AFFINITY retain cache + // NO_PREFERENCE do not retain cache + // If queryDataCacheEnabledDefault is false, when `node_selection_strategy` + // is + // not set do not retain cache + // SOFT_AFFINITY retain cache + // NO_PREFERENCE do not retain cache + connectorConfig.emplace( + connector::hive::HiveConfig::kCacheNoRetentionSession, + systemConfig->queryDataCacheEnabledDefault() ? "false" : "true"); auto it = connectorConfig.find("node_selection_strategy"); - if (it != connectorConfig.end() && it->second == "NO_PREFERENCE") { - connectorConfig.emplace( - connector::hive::HiveConfig::kCacheNoRetentionSession, "true"); + if (it != connectorConfig.end()) { + connectorConfig[connector::hive::HiveConfig::kCacheNoRetentionSession] = + it->second == "SOFT_AFFINITY" ? "false" : "true"; } } } diff --git a/presto-native-execution/presto_cpp/main/common/Configs.cpp b/presto-native-execution/presto_cpp/main/common/Configs.cpp index 0dd6780e19e33..dfd37e507164f 100644 --- a/presto-native-execution/presto_cpp/main/common/Configs.cpp +++ b/presto-native-execution/presto_cpp/main/common/Configs.cpp @@ -178,6 +178,7 @@ SystemConfig::SystemConfig() { BOOL_PROP(kNativeSidecar, false), BOOL_PROP(kAsyncDataCacheEnabled, true), NUM_PROP(kAsyncCacheSsdGb, 0), + BOOL_PROP(kQueryDataCacheEnabledDefault, true), NUM_PROP(kAsyncCacheSsdCheckpointGb, 0), STR_PROP(kAsyncCacheSsdPath, "/mnt/flash/async_cache."), NUM_PROP(kAsyncCacheMaxSsdWriteRatio, 0.7), @@ -464,6 +465,10 @@ bool SystemConfig::asyncDataCacheEnabled() const { return optionalProperty(kAsyncDataCacheEnabled).value(); } +bool SystemConfig::queryDataCacheEnabledDefault() const { + return optionalProperty(kQueryDataCacheEnabledDefault).value(); +} + uint64_t SystemConfig::asyncCacheSsdCheckpointGb() const { return optionalProperty(kAsyncCacheSsdCheckpointGb).value(); } diff --git a/presto-native-execution/presto_cpp/main/common/Configs.h b/presto-native-execution/presto_cpp/main/common/Configs.h index 25cdfa13d265e..77cfa1feac6ab 100644 --- a/presto-native-execution/presto_cpp/main/common/Configs.h +++ b/presto-native-execution/presto_cpp/main/common/Configs.h @@ -313,6 +313,12 @@ class SystemConfig : public ConfigBase { static constexpr std::string_view kAsyncDataCacheEnabled{ "async-data-cache-enabled"}; + /// If true, SSD cache is enabled by default and is disabled only if + /// `node_selection_strategy` is present and set to `NO_PREFERENCE`. + /// Otherwise, SSD cache is disabled by default and is enabled if + /// `node_selection_strategy` is present and set to `SOFT_AFFINITY`. + static constexpr std::string_view kQueryDataCacheEnabledDefault{ + "query-data-cache-enabled-default"}; static constexpr std::string_view kAsyncCacheSsdGb{"async-cache-ssd-gb"}; static constexpr std::string_view kAsyncCacheSsdCheckpointGb{ "async-cache-ssd-checkpoint-gb"}; @@ -752,6 +758,8 @@ class SystemConfig : public ConfigBase { bool asyncDataCacheEnabled() const; + bool queryDataCacheEnabledDefault() const; + uint64_t asyncCacheSsdGb() const; uint64_t asyncCacheSsdCheckpointGb() const;