Skip to content

Commit 269ebb8

Browse files
HeidiHan0000facebook-github-bot
authored andcommitted
Add spill related configs to system configs
Summary: Adding spill configs so they can be set from configs, and then overridden by session properties. Differential Revision: D71002997
1 parent fa9a268 commit 269ebb8

File tree

4 files changed

+89
-3
lines changed

4 files changed

+89
-3
lines changed

presto-native-execution/presto_cpp/main/QueryContextManager.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,15 @@ void updateFromSystemConfigs(
4545
{core::QueryConfig::kQueryMaxMemoryPerNode,
4646
std::string(SystemConfig::kQueryMaxMemoryPerNode)},
4747
{core::QueryConfig::kSpillFileCreateConfig,
48-
std::string(SystemConfig::kSpillerFileCreateConfig)}};
48+
std::string(SystemConfig::kSpillerFileCreateConfig)},
49+
{core::QueryConfig::kSpillEnabled,
50+
std::string(SystemConfig::kSpillEnabled)},
51+
{core::QueryConfig::kJoinSpillEnabled,
52+
std::string(SystemConfig::kJoinSpillEnabled)},
53+
{core::QueryConfig::kOrderBySpillEnabled,
54+
std::string(SystemConfig::kOrderBySpillEnabled)},
55+
{core::QueryConfig::kAggregationSpillEnabled,
56+
std::string(SystemConfig::kAggregationSpillEnabled)}};
4957

5058
for (const auto& configNameEntry : sessionSystemConfigMapping) {
5159
const auto& sessionName = configNameEntry.first;

presto-native-execution/presto_cpp/main/common/Configs.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ SystemConfig::SystemConfig() {
250250
BOOL_PROP(kPlanValidatorFailOnNestedLoopJoin, false),
251251
STR_PROP(kPrestoDefaultNamespacePrefix, "presto.default"),
252252
STR_PROP(kPoolType, "DEFAULT"),
253+
BOOL_PROP(kSpillEnabled, false),
254+
BOOL_PROP(kJoinSpillEnabled, true),
255+
BOOL_PROP(kAggregationSpillEnabled, true),
256+
BOOL_PROP(kOrderBySpillEnabled, true),
253257
};
254258
}
255259

@@ -313,6 +317,22 @@ std::string SystemConfig::poolType() const {
313317
return value;
314318
}
315319

320+
bool SystemConfig::spillEnabled() const {
321+
return optionalProperty<bool>(kSpillEnabled).value();
322+
}
323+
324+
bool SystemConfig::joinSpillEnabled() const {
325+
return optionalProperty<bool>(kJoinSpillEnabled).value();
326+
}
327+
328+
bool SystemConfig::aggregationSpillEnabled() const {
329+
return optionalProperty<bool>(kAggregationSpillEnabled).value();
330+
}
331+
332+
bool SystemConfig::orderBySpillEnabled() const {
333+
return optionalProperty<bool>(kOrderBySpillEnabled).value();
334+
}
335+
316336
bool SystemConfig::mutableConfig() const {
317337
return optionalProperty<bool>(kMutableConfig).value();
318338
}

presto-native-execution/presto_cpp/main/common/Configs.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,14 @@ class SystemConfig : public ConfigBase {
662662

663663
// Specifies the type of worker pool
664664
static constexpr std::string_view kPoolType{"pool-type"};
665+
666+
// Spill related configs
667+
static constexpr std::string_view kSpillEnabled{"spill-enabled"};
668+
static constexpr std::string_view kJoinSpillEnabled{"join-spill-enabled"};
669+
static constexpr std::string_view kAggregationSpillEnabled{
670+
"aggregation-spill-enabled"};
671+
static constexpr std::string_view kOrderBySpillEnabled{
672+
"order-by-spill-enabled"};
665673

666674
SystemConfig();
667675

@@ -904,6 +912,14 @@ class SystemConfig : public ConfigBase {
904912
std::string prestoDefaultNamespacePrefix() const;
905913

906914
std::string poolType() const;
915+
916+
bool spillEnabled() const;
917+
918+
bool joinSpillEnabled() const;
919+
920+
bool aggregationSpillEnabled() const;
921+
922+
bool orderBySpillEnabled() const;
907923
};
908924

909925
/// Provides access to node properties defined in node.properties file.

presto-native-execution/presto_cpp/main/tests/QueryContextManagerTest.cpp

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,34 +108,54 @@ TEST_F(QueryContextManagerTest, defaultSessionProperties) {
108108
EXPECT_EQ(queryConfig.maxSpillLevel(), defaultQC->maxSpillLevel());
109109
EXPECT_EQ(
110110
queryConfig.spillCompressionKind(), defaultQC->spillCompressionKind());
111+
EXPECT_EQ(queryConfig.spillEnabled(), defaultQC->spillEnabled());
112+
EXPECT_EQ(queryConfig.aggregationSpillEnabled(), defaultQC->aggregationSpillEnabled());
111113
EXPECT_EQ(queryConfig.joinSpillEnabled(), defaultQC->joinSpillEnabled());
114+
EXPECT_EQ(queryConfig.orderBySpillEnabled(), defaultQC->orderBySpillEnabled());
112115
EXPECT_EQ(
113116
queryConfig.validateOutputFromOperators(),
114117
defaultQC->validateOutputFromOperators());
115118
EXPECT_EQ(
116119
queryConfig.spillWriteBufferSize(), defaultQC->spillWriteBufferSize());
117120
}
118121

119-
TEST_F(QueryContextManagerTest, overrdingSessionProperties) {
122+
TEST_F(QueryContextManagerTest, overridingSessionProperties) {
120123
protocol::TaskId taskId = "scan.0.0.1.0";
121124
const auto& systemConfig = SystemConfig::instance();
122125
{
123126
protocol::SessionRepresentation session{.systemProperties = {}};
124127
auto queryCtx =
125128
taskManager_->getQueryContextManager()->findOrCreateQueryCtx(
126129
taskId, session);
130+
// When session properties are not explicitly set, they should be set to
131+
// system config values.
127132
EXPECT_EQ(
128133
queryCtx->queryConfig().queryMaxMemoryPerNode(),
129134
systemConfig->queryMaxMemoryPerNode());
130135
EXPECT_EQ(
131136
queryCtx->queryConfig().spillFileCreateConfig(),
132137
systemConfig->spillerFileCreateConfig());
138+
EXPECT_EQ(
139+
queryCtx->queryConfig().spillEnabled(),
140+
systemConfig->spillEnabled());
141+
EXPECT_EQ(
142+
queryCtx->queryConfig().aggregationSpillEnabled(),
143+
systemConfig->aggregationSpillEnabled());
144+
EXPECT_EQ(
145+
queryCtx->queryConfig().joinSpillEnabled(),
146+
systemConfig->joinSpillEnabled());
147+
EXPECT_EQ(
148+
queryCtx->queryConfig().orderBySpillEnabled(),
149+
systemConfig->orderBySpillEnabled());
133150
}
134151
{
135152
protocol::SessionRepresentation session{
136153
.systemProperties = {
137154
{"query_max_memory_per_node", "1GB"},
138-
{"spill_file_create_config", "encoding:replica_2"}}};
155+
{"spill_file_create_config", "encoding:replica_2"},
156+
{"spill_enabled", "true"},
157+
{"aggregation_spill_enabled", "false"},
158+
{"join_spill_enabled", "true"}}};
139159
auto queryCtx =
140160
taskManager_->getQueryContextManager()->findOrCreateQueryCtx(
141161
taskId, session);
@@ -144,6 +164,28 @@ TEST_F(QueryContextManagerTest, overrdingSessionProperties) {
144164
1UL * 1024 * 1024 * 1024);
145165
EXPECT_EQ(
146166
queryCtx->queryConfig().spillFileCreateConfig(), "encoding:replica_2");
167+
// Override with different value
168+
EXPECT_EQ(
169+
queryCtx->queryConfig().spillEnabled(), true);
170+
EXPECT_NE(
171+
queryCtx->queryConfig().spillEnabled(),
172+
systemConfig->spillEnabled());
173+
// Override with different value
174+
EXPECT_EQ(
175+
queryCtx->queryConfig().aggregationSpillEnabled(), false);
176+
EXPECT_NE(
177+
queryCtx->queryConfig().aggregationSpillEnabled(),
178+
systemConfig->aggregationSpillEnabled());
179+
// Override with same value
180+
EXPECT_EQ(
181+
queryCtx->queryConfig().joinSpillEnabled(), true);
182+
EXPECT_EQ(
183+
queryCtx->queryConfig().joinSpillEnabled(),
184+
systemConfig->joinSpillEnabled());
185+
// No override
186+
EXPECT_EQ(
187+
queryCtx->queryConfig().orderBySpillEnabled(),
188+
systemConfig->orderBySpillEnabled());
147189
}
148190
}
149191

0 commit comments

Comments
 (0)