-
Notifications
You must be signed in to change notification settings - Fork 7
Preserve parquet specific parallel formatting for export part #1106
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
|
maybe it is better to simply store the context ptr? the only question is: can it expire or smth of the sort? |
| result += raw_path; | ||
|
|
||
| if (raw_path.back() != '/') | ||
| if (!raw_path.empty() && raw_path.back() != '/') |
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.
Unrelated but required
|
Not tested yet |
|
Tested with debugger, checked that max_threads, parquet parallel formatting and parallel formatting are kept |
| break; | ||
| case Export: | ||
| read_settings.local_throttler = context->getExportsThrottler(); | ||
| addThrottler(read_settings.local_throttler, context->getExportsThrottler()); |
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.
Merge glitch, only found it now
|
tested and confirm that it works |
| bool overwrite_file_if_exists; | ||
| bool parallel_formatting; | ||
| /// parquet has a different setting for parallel formatting | ||
| bool parallel_formatting_parquet; |
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.
Maybe rename that one to be similar to the setting name? something like
| bool parallel_formatting_parquet; | |
| bool parquet_parallel_encoding; |
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.
Done
|
03572_export_merge_tree_part_to_object_storage - maybe it is flaky, I don't see how this change could break it |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Preserve a few file format settings in the export part manifest to be able to better control parallelism
Documentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: