Skip to content

Conversation

@arthurpassos
Copy link
Collaborator

@arthurpassos arthurpassos commented Oct 28, 2025

Changelog category (leave one):

  • Improvement

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:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • Tiered Storage (2h)

@arthurpassos
Copy link
Collaborator Author

maybe it is better to simply store the context ptr? the only question is: can it expire or smth of the sort?

@github-actions
Copy link

github-actions bot commented Oct 28, 2025

Workflow [PR], commit [8238426]

@arthurpassos arthurpassos changed the title preserve parquet specific parallel formatting for export part Preserve parquet specific parallel formatting for export part Oct 29, 2025
@arthurpassos arthurpassos marked this pull request as ready for review October 29, 2025 09:51
result += raw_path;

if (raw_path.back() != '/')
if (!raw_path.empty() && raw_path.back() != '/')
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated but required

@arthurpassos
Copy link
Collaborator Author

Not tested yet

@arthurpassos
Copy link
Collaborator Author

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());
Copy link
Collaborator Author

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

@dima-altinity
Copy link

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;
Copy link
Member

@Enmk Enmk Nov 3, 2025

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

Suggested change
bool parallel_formatting_parquet;
bool parquet_parallel_encoding;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@arthurpassos
Copy link
Collaborator Author

03572_export_merge_tree_part_to_object_storage - maybe it is flaky, I don't see how this change could break it

@Enmk Enmk merged commit 60c312c into antalya-25.8 Nov 6, 2025
136 of 138 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants