Antalya 26.3: apassos-2: combined port of 12 PRs#1699
Closed
zvonand wants to merge 4 commits intoantalya-26.3from
Closed
Antalya 26.3: apassos-2: combined port of 12 PRs#1699zvonand wants to merge 4 commits intoantalya-26.3from
zvonand wants to merge 4 commits intoantalya-26.3from
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Forward port of export part and partition #1041, #1083, #1086, #1090, #1106, #1124, #1144, #1147, #1150, #1157, #1158, #1161, #1167, #1229, #1294, #1320, #1324 and #1330 (#1388 by @arthurpassos, #1405 by @arthurpassos, #1478 by @arthurpassos, #1402 by @arthurpassos, #1484 by @arthurpassos, #1490 by @arthurpassos, #1500 by @arthurpassos, #1517 by @arthurpassos, #1499 by @arthurpassos, #1593 by @arthurpassos, #1618 by @arthurpassos, #1631 by @arthurpassos).
CI/CD Options
Exclude tests:
Regression jobs to run:
Combined port of 12 PR(s) (group
apassos-2). Cherry-picked from #1388, #1405, #1478, #1402, #1484, #1490, #1500, #1517, #1499, #1593, #1618, #1631.#1388: Antalya 26.1 - Forward port of export part and partition
Documentation entry for user-facing changes
Export merge tree part and partition (we still need to rebase #1177 afterwards)
#1405: Antalya 26.1 - Forward port of list objects cache #1040
Documentation entry for user-facing changes
Cache for listobjects calls
#1478: Skip remote suite on export tests that block minio
Documentation entry for user-facing changes
...
#1402: Improvements to partition export
Documentation entry for user-facing changes
As of now, this PR does the following things:
solvesthe problem of one replica locking all parts in a task just because it ran faster even if it did not have bandwidth to execute all of them.ExportPartFromPartitionExportTasksystem.replicated_partition_exportsoption. Refactor queryingsystem.replicated_partition_exportsto usemulti_readrequests instead of several different read requests. Throws iffmulti_readis not supported (this was vibe coded).Settingsobject instead of onlyFormatSettings- now more settings should be preserved (part export only)lock_inside_the_taskis not production ready as of now (the entire feature is not production ready tbh) - there is a possible crash in case the user schedules an export, changes the schema of the destination table, and the export executes. This is because validation is not being done on the fly for this setting. But I think it is ok to ignore this corner case for now.This partially tackles the following:
#1484: Use serialized metadata size to calculate the cache entry cell
Documentation entry for user-facing changes
...
#1490: add setting to define filename pattern for part exports
Documentation entry for user-facing changes
...
#1500: Fix local replicated_partition_exports table might miss entries
Documentation entry for user-facing changes
...
#1517: Fix IPartitionStrategy race condition
IPartitionStrategy::computePartitionKey might be called from different threads, and it writes to cached_result concurrently without any sort of protection. It would be easier to add a mutex around it, but we can actually make it lock-free by moving the cache write to the constructor.
Documentation entry for user-facing changes
...
#1499: Bump scheduled exports count only in case it has been scheduled
Documentation entry for user-facing changes
...
#1593: Export Partition - release the part lock when the query is cancelled
During export partition, parts are locked by replicas for exports. This PR introduces a change that releases these locks when an export task is cancelled. Previously, it would not release the lock. We did not catch this error before because the only cases an export task was cancelled we tested were
KILL EXPORT PARTITIONandDROP TABLE. In those cases, the entire task is cancelled, so it does not matter if a replica does not release its lock.But a query can also be cancelled with 'SYSTEM STOP MOVES', and in that case, it is a local operation. The lock must be released so other replicas can continue.
Documentation entry for user-facing changes
...
#1618: Export partition to apache iceberg
Export partition mechanics changes:
export_merge_tree_partition_system_table_prefer_remote_informationfalse by default (I am considering to remove it completely)getContextCopyWithTaskSettingsto avoid code duplicationenable_experimental_export_merge_tree_partition_featuretoallow_experimental_export_merge_tree_partitionallow_experimental_insert_into_icebergnot enabledApache Iceberg specifics:
write_full_path_in_iceberg_metadatain zookeeper taskf_clickhouse_export_partition_transaction_idto apache iceberg manifest so we can check it before comitting twiceDocumentation entry for user-facing changes
...
#1631: Fix condition for using parquet metadata cache
Apache Iceberg queries were not htiting the parquet metadata cache because
object_info->getFileFormat()resolves toIcebergDataObjectInfo::getFileFormat, which gets its return value fromIcebergObjectSerializableInfo. This field is filled with the value from Apache Iceberg manifest file, and it is upper case by default, which then fails clickhouse check for parquet metadata cache usage.Documentation entry for user-facing changes
...