Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[VL] Query hangs on destroying memory manager #4232

Merged
merged 5 commits into from
Dec 29, 2023
Merged

[VL] Query hangs on destroying memory manager #4232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe7cc58
fixup
zhztheplayer Dec 29, 2023
202140c
fixup
zhztheplayer Dec 29, 2023
0e301a8
fixup
zhztheplayer Dec 29, 2023
138ce1f
fixup
zhztheplayer Dec 29, 2023
0d63fe3
fixup
zhztheplayer Dec 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cpp/velox/compute/VeloxRuntime.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ std::shared_ptr<Datasource> VeloxRuntime::createDatasource(
const std::string& filePath,
MemoryManager* memoryManager,
std::shared_ptr<arrow::Schema> schema) {
auto veloxPool = getAggregateVeloxPool(memoryManager);
static std::atomic_uint32_t id{0UL};
auto veloxPool = getAggregateVeloxPool(memoryManager)->addAggregateChild("datasource." + std::to_string(id++));
// Pass a dedicate pool for S3 sink as can't share veloxPool
// with parquet writer.
auto s3SinkPool = getLeafVeloxPool(memoryManager);
Expand Down
21 changes: 19 additions & 2 deletions cpp/velox/memory/VeloxMemoryManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,25 @@ bool VeloxMemoryManager::tryDestructSafe() {
veloxAggregatePool_.reset();

// Velox memory manager considered safe to destruct when no alive pools.
if (veloxMemoryManager_ && veloxMemoryManager_->numPools() != 0) {
return false;
if (veloxMemoryManager_) {
if (veloxMemoryManager_->numPools() > 1) {
return false;
}
if (veloxMemoryManager_->numPools() == 1) {
// Assert the pool is spill pool
// See https://github.com/facebookincubator/velox/commit/e6f84e8ac9ef6721f527a2d552a13f7e79bdf72e
int32_t spillPoolCount = 0;
veloxMemoryManager_->testingDefaultRoot().visitChildren([&](velox::memory::MemoryPool* child) -> bool {
if (child == veloxMemoryManager_->spillPool()) {
spillPoolCount++;
}
return true;
});
GLUTEN_CHECK(spillPoolCount == 1, "Illegal pool count state: spillPoolCount: " + std::to_string(spillPoolCount));
}
if (veloxMemoryManager_->numPools() < 1) {
GLUTEN_CHECK(false, "Unreachable code");
}
}
veloxMemoryManager_.reset();

Expand Down
4 changes: 4 additions & 0 deletions gluten-ut/spark34/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,10 @@ class VeloxTestSettings extends BackendTestSettings {
// error message mismatch is accepted
.exclude("schema mismatch failure error message for parquet reader")
.exclude("schema mismatch failure error message for parquet vectorized reader")
// [PATH_NOT_FOUND] Path does not exist:
// file:/opt/spark331/sql/core/src/test/resources/test-data/timestamp-nanos.parquet
// May require for newer spark.test.home
.excludeByPrefix("SPARK-40819")
enableSuite[GlutenParquetThriftCompatibilitySuite]
// Rewrite for file locating.
.exclude("Read Parquet file generated by parquet-thrift")
Expand Down
Loading