-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[native] Add session properties for scale writer query configs #24244
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
Conversation
We also need to add them to coordinator Can we squash the commits to one? |
On the coordinator side, the native session properties have been moved to |
Thanks, @pdabre12 . However, I noticed that NativeWorkerSessionPropertyProvider is marked as deprecated, and there doesn't seem to be any replacement suggestion. This is causing some confusion. Should we remove the deprecated annotation or do you have any recommendations? |
# Conflicts: # presto-native-execution/presto_cpp/main/SessionProperties.h
It is marked as deprecated, because we support the retrieval of the session properties through the sidecar, the PR: :#23045. Ideally, we want to use this functionality but this is a temporary class where native session properties were re-located to until the adoption of the sidecar. |
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.
lgtm
…bytes_rebalance_threshold
…bytes_rebalance_threshold
Saved that user @duxiao1212 is from Meta |
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.
@duxiao1212 thanks for the change!
Description
Motivation and Context
Add session properties for scale writer query configs
Impact
Low impact
Contributor checklist