From ee92b3103bc2e6b868b65947bbc11e10caf996ee Mon Sep 17 00:00:00 2001 From: Jialiang Tan Date: Mon, 24 Jul 2023 16:02:13 -0700 Subject: [PATCH] =?UTF-8?q?Remove=20inheritence=20relationship=20between?= =?UTF-8?q?=20AsyncDataCache=20and=20MemoryAllo=E2=80=A6=20(#20372)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pull Request resolved: https://github.com/prestodb/presto/pull/20372 Removing the relationship of AsyncDataCache inheritance from MemoryAllocator. Now they are depending on each other with registration mechanism. Related tests are refactored to be consistent with the new change. X-link: https://github.com/facebookincubator/velox/pull/5503 Reviewed By: xiaoxmeng Differential Revision: D47536273 Pulled By: tanjialiang fbshipit-source-id: 7749b787bf2a1027b33e02690917c59bb497026c --- .../presto_cpp/main/PrestoServer.cpp | 8 +++++--- .../presto_cpp/main/PrestoServer.h | 11 +++-------- .../presto_cpp/main/QueryContextManager.cpp | 2 +- .../presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp | 6 +++--- presto-native-execution/velox | 2 +- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/PrestoServer.cpp b/presto-native-execution/presto_cpp/main/PrestoServer.cpp index eec870ed97394..84bff2c30e2bf 100644 --- a/presto-native-execution/presto_cpp/main/PrestoServer.cpp +++ b/presto-native-execution/presto_cpp/main/PrestoServer.cpp @@ -434,6 +434,9 @@ void PrestoServer::run() { << "': threads: " << pGlobalIOExecutor->numActiveThreads() << "/" << pGlobalIOExecutor->numThreads(); } + + PRESTO_SHUTDOWN_LOG(INFO) << "Release resources in AsyncDataCache"; + cache_->prepareShutdown(); } void PrestoServer::yieldTasks() { @@ -492,9 +495,8 @@ void PrestoServer::initializeVeloxMemory() { asyncCacheSsdCheckpointGb << 30, asyncCacheSsdDisableFileCow); } - cache_ = std::make_shared( - allocator_, memoryBytes, std::move(ssd)); - allocator_ = cache_; + cache_ = cache::AsyncDataCache::create(allocator_.get(), std::move(ssd)); + cache::AsyncDataCache::setInstance(cache_.get()); } else { VELOX_CHECK_EQ( systemConfig->asyncCacheSsdGb(), diff --git a/presto-native-execution/presto_cpp/main/PrestoServer.h b/presto-native-execution/presto_cpp/main/PrestoServer.h index 0fd188ed4b5d8..d554a04a2dc2d 100644 --- a/presto-native-execution/presto_cpp/main/PrestoServer.h +++ b/presto-native-execution/presto_cpp/main/PrestoServer.h @@ -165,17 +165,12 @@ class PrestoServer { // Executor for exchange data over http. std::shared_ptr exchangeExecutor_; - // If not null, the instance of AsyncDataCache used for in-memory file cache. - std::shared_ptr cache_; - // Instance of MemoryAllocator used for all query memory allocations. - // - // NOTE: AsyncDataCache implements MemoryAllocator interface and wraps on top - // of a real memory allocator for the actual memory allocation. So if 'cache_' - // has been set, 'allocator_' is also set to 'cache_'. It provides memory - // allocations for both file cache and query memory. std::shared_ptr allocator_; + // If not null, the instance of AsyncDataCache used for in-memory file cache. + std::shared_ptr cache_; + std::unique_ptr httpServer_; std::unique_ptr signalHandler_; std::unique_ptr announcer_; diff --git a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp index 4fc69c2db7e4b..c07e8e39d82ea 100644 --- a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp +++ b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp @@ -135,7 +135,7 @@ std::shared_ptr QueryContextManager::findOrCreateQueryCtx( executor().get(), std::move(configStrings), connectorConfigs, - memory::MemoryAllocator::getInstance(), + cache::AsyncDataCache::getInstance(), std::move(pool), spillExecutor(), queryId); diff --git a/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp b/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp index a20a3eace53b8..d02144f19e96b 100644 --- a/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp +++ b/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp @@ -2574,9 +2574,9 @@ core::PlanFragment VeloxQueryPlanConverterBase::toVeloxQueryPlan( planFragment.planNode = std::make_shared( "root", + core::PartitionedOutputNode::Kind::kPartitioned, partitioningKeys, numPartitions, - core::PartitionedOutputNode::Kind::kPartitioned, partitioningScheme.replicateNullsAndAny, std::make_shared(), outputType, @@ -2594,9 +2594,9 @@ core::PlanFragment VeloxQueryPlanConverterBase::toVeloxQueryPlan( planFragment.planNode = std::make_shared( "root", + core::PartitionedOutputNode::Kind::kPartitioned, partitioningKeys, numPartitions, - core::PartitionedOutputNode::Kind::kPartitioned, partitioningScheme.replicateNullsAndAny, std::make_shared( inputType, keyChannels, constValues), @@ -2642,9 +2642,9 @@ core::PlanFragment VeloxQueryPlanConverterBase::toVeloxQueryPlan( planFragment.planNode = std::make_shared( "root", + core::PartitionedOutputNode::Kind::kPartitioned, partitioningKeys, numPartitions, - core::PartitionedOutputNode::Kind::kPartitioned, partitioningScheme.replicateNullsAndAny, std::make_shared( hivePartitioningHandle->bucketCount, diff --git a/presto-native-execution/velox b/presto-native-execution/velox index 273679c06e4df..6516bbba0e792 160000 --- a/presto-native-execution/velox +++ b/presto-native-execution/velox @@ -1 +1 @@ -Subproject commit 273679c06e4dfcf47c3af6270a76b2b1f38ee367 +Subproject commit 6516bbba0e7925730376197b358f5a746c8a308d