Skip to content

Commit 342d25a

Browse files
committed
fix: prevent zero-thread DefaultExecutor and validate FileSystem/SchemeMap conflict
1. DefaultExecutor: guard against thread_count==0 by falling back to 1 2. GetGlobalDefaultExecutor: handle hardware_concurrency()==0 case 3. ReadContextBuilder::Finish: validate prefetch_max_parallel_num > 0 4. ReadContextBuilder::Finish: reject simultaneous WithFileSystem() and WithFileSystemSchemeToIdentifierMap() as documented in API contract 5. GlobalIndexScanImpl::Create: guard against global-index.thread-num <= 0 6. Update read_context_test to match new validation rules
1 parent c1b5a27 commit 342d25a

4 files changed

Lines changed: 40 additions & 5 deletions

File tree

src/paimon/common/executor/executor.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ class DefaultExecutor : public Executor {
4949
int32_t active_tasks_ = 0;
5050
};
5151

52-
DefaultExecutor::DefaultExecutor(uint32_t thread_count) : thread_count_(thread_count) {
52+
DefaultExecutor::DefaultExecutor(uint32_t thread_count)
53+
: thread_count_(thread_count == 0 ? 1 : thread_count) {
5354
for (uint32_t i = 0; i < thread_count_; ++i) {
5455
workers_.emplace_back(&DefaultExecutor::WorkerThread, this);
5556
}
@@ -130,7 +131,7 @@ void DefaultExecutor::WorkerThread() {
130131
PAIMON_EXPORT std::shared_ptr<Executor> GetGlobalDefaultExecutor() {
131132
static uint32_t all_cores = std::thread::hardware_concurrency();
132133
static std::shared_ptr<Executor> internal =
133-
std::make_shared<DefaultExecutor>(/*thread_count=*/all_cores);
134+
std::make_shared<DefaultExecutor>(/*thread_count=*/all_cores > 0 ? all_cores : 1);
134135
return internal;
135136
}
136137

src/paimon/core/global_index/global_index_scan_impl.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ Result<std::unique_ptr<GlobalIndexScanImpl>> GlobalIndexScanImpl::Create(
107107
uint32_t cpu_count = std::thread::hardware_concurrency();
108108
thread_num = cpu_count > 0 ? static_cast<int32_t>(cpu_count) : 1;
109109
}
110+
if (thread_num.value() <= 0) {
111+
thread_num = 1;
112+
}
110113
final_executor = CreateDefaultExecutor(static_cast<uint32_t>(thread_num.value()));
111114
}
112115
return std::unique_ptr<GlobalIndexScanImpl>(new GlobalIndexScanImpl(

src/paimon/core/operation/read_context.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ Result<std::unique_ptr<ReadContext>> ReadContextBuilder::Finish() {
222222
if (impl_->path_.empty()) {
223223
return Status::Invalid("cannot read with empty table path");
224224
}
225+
if (impl_->enable_prefetch_ && impl_->prefetch_max_parallel_num_ == 0) {
226+
return Status::Invalid("prefetch max parallel num should be greater than 0");
227+
}
225228
if (impl_->enable_prefetch_ && impl_->prefetch_batch_count_ <= 0) {
226229
return Status::Invalid("prefetch batch count should be greater than 0");
227230
}
@@ -230,6 +233,10 @@ Result<std::unique_ptr<ReadContext>> ReadContextBuilder::Finish() {
230233
return Status::Invalid(
231234
"prefetch batch count should be greater than or equal to prefetch max parallel num");
232235
}
236+
if (impl_->specific_file_system_ && !impl_->fs_scheme_to_identifier_map_.empty()) {
237+
return Status::Invalid(
238+
"WithFileSystem() and WithFileSystemSchemeToIdentifierMap() cannot be used together");
239+
}
233240
if (!impl_->executor_) {
234241
// If the user do not set executor, create default executor by prefetch batch count
235242
uint32_t thread_count = impl_->enable_prefetch_ ? impl_->prefetch_max_parallel_num_ : 1;

src/paimon/core/operation/read_context_test.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ TEST(ReadContextTest, TestSetContent) {
7575
builder.SetTableSchema("table-schema-json");
7676
builder.WithBranch("rt");
7777
builder.WithCacheConfig(cache_config);
78-
builder.WithFileSystemSchemeToIdentifierMap({{"file", "local"}});
7978
auto fs = std::make_shared<MockFileSystem>();
8079
builder.WithFileSystem(fs);
8180
ASSERT_OK_AND_ASSIGN(auto ctx, builder.Finish());
@@ -103,8 +102,7 @@ TEST(ReadContextTest, TestSetContent) {
103102
ASSERT_EQ(512U, ctx->GetCacheConfig().GetRangeSizeLimit());
104103
ASSERT_EQ(128U, ctx->GetCacheConfig().GetHoleSizeLimit());
105104
ASSERT_EQ(2048U, ctx->GetCacheConfig().GetPreBufferLimit());
106-
std::map<std::string, std::string> expected_fs_map = {{"file", "local"}};
107-
ASSERT_EQ(expected_fs_map, ctx->GetFileSystemSchemeToIdentifierMap());
105+
ASSERT_TRUE(ctx->GetFileSystemSchemeToIdentifierMap().empty());
108106
std::map<std::string, std::string> expected_options = {{"key", "value"}};
109107
ASSERT_EQ(expected_options, ctx->GetOptions());
110108
ASSERT_EQ(ctx->GetSpecificFileSystem(), fs);
@@ -121,4 +119,30 @@ TEST(ReadContextTest, TestSetOptionsOverridesAddedOptions) {
121119
ASSERT_EQ(expected_options, ctx->GetOptions());
122120
}
123121

122+
TEST(ReadContextTest, TestFileSystemAndSchemeMapConflict) {
123+
ReadContextBuilder builder("table_root_path");
124+
auto fs = std::make_shared<MockFileSystem>();
125+
builder.WithFileSystem(fs);
126+
builder.WithFileSystemSchemeToIdentifierMap({{"file", "local"}});
127+
ASSERT_NOK_WITH_MSG(
128+
builder.Finish(),
129+
"WithFileSystem() and WithFileSystemSchemeToIdentifierMap() cannot be used together");
130+
}
131+
132+
TEST(ReadContextTest, TestSchemeMapWithoutFileSystem) {
133+
ReadContextBuilder builder("table_root_path");
134+
builder.WithFileSystemSchemeToIdentifierMap({{"file", "local"}});
135+
ASSERT_OK_AND_ASSIGN(auto ctx, builder.Finish());
136+
std::map<std::string, std::string> expected_fs_map = {{"file", "local"}};
137+
ASSERT_EQ(expected_fs_map, ctx->GetFileSystemSchemeToIdentifierMap());
138+
ASSERT_FALSE(ctx->GetSpecificFileSystem());
139+
}
140+
141+
TEST(ReadContextTest, TestPrefetchMaxParallelNumZero) {
142+
ReadContextBuilder builder("table_root_path");
143+
builder.EnablePrefetch(true);
144+
builder.SetPrefetchMaxParallelNum(0);
145+
ASSERT_NOK_WITH_MSG(builder.Finish(), "prefetch max parallel num should be greater than 0");
146+
}
147+
124148
} // namespace paimon::test

0 commit comments

Comments
 (0)