-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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.
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
[native] cache.no_retention should also be false when hive.node_selection_strategy=HARD_AFFINITY #24147
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@minhancao Can you please clean up the release note section?
@@ -117,7 +117,9 @@ void updateVeloxConnectorConfigs( | |||
auto it = connectorConfig.find("node_selection_strategy"); | |||
if (it != connectorConfig.end()) { | |||
connectorConfig[connector::hive::HiveConfig::kCacheNoRetentionSession] = | |||
it->second == "SOFT_AFFINITY" ? "false" : "true"; | |||
(it->second == "SOFT_AFFINITY" || it->second == "HARD_AFFINITY") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about it->second != "SOFT_AFFINITY" && it->second != "HARD_AFFINITY"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing logic to:
connectorConfig[connector::hive::HiveConfig::kCacheNoRetentionSession] =
(it->second != "SOFT_AFFINITY" && it->second != "HARD_AFFINITY") ? "true" : "false";
fcd074c
to
6f34571
Compare
…tegy=HARD_AFFINITY
Description
Previous PR below allows setting hive.node_selection_strategy=SOFT_AFFINITY to set cache.no_retention=FALSE.
#24076
This PR adds setting hive.node_selection_strategy=HARD_AFFINITY will set cache.no_retention=FALSE as well.
Motivation and Context
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.