Skip to content

feat(clickhouse): Skip past-retention partitions in attach-partitions - #8567

Merged
onewland merged 1 commit into
masterfrom
oliver/attach-skip-expired-partitions
Oct 19, 2026
Merged

onewland merged 1 commit into
masterfrom
oliver/attach-skip-expired-partitions

Conversation

@onewland

@onewland onewland commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

attach-partitions now skips source partitions that are already past retention, using the same rule prune-partitions uses to decide a partition is dead.

Until now, discovery mode only had an upper date bound (--max-future-days). Every past partition with active source parts was attached, including long-expired weeks and garbage partitions such as 90-19700101. That causes two problems:

  • Wasted work: destination TTL deletes the attached rows anyway.

What changes:

  • Skip rule: a partition is skipped when classify_partition from prune_partitions.py returns dead_past, meaning week_start + 7d + retention_days + --retention-margin-days (default 14) is before today. Both commands use that one function, so they can't disagree about what's expired.
  • Weekly keys only: the filter applies only when the source table is partitioned by (retention_days, toMonday(timestamp)). A key like (retention_days, toStartOfMonth(...)) renders the same 90-20260601 ID shape but isn't weekly, so for any other key every partition is kept. The partition key is only read from system.tables when some candidate ID looks expired, so tables with other ID formats don't pay for an extra query.
  • Opt-out: --include-expired restores the old behaviour.
  • Single partition: a single --partition-id that is past retention is refused unless --include-expired is passed, matching how --max-future-days treats a future partition.
  • Small refactor in prune_partitions.py: the partition-key regex is exposed as is_weekly_retention_partition_key. prune-partitions behaves the same.

For reviewers:

  • Default behaviour changes. Discovery runs that used to attach old weeks will now skip them and log Skipping partition … past retention. Pass --include-expired if a backfill really needs expired data.
  • Two existing tests changed. They used 2024 partition IDs with no now, so they now pass retention_margin=None to keep testing boundary and health-check scoping without depending on today's date.

🤖 Generated with Claude Code

attach-partitions had an upper date bound (--max-future-days) but no lower
one, so it attached source partitions that were already past retention,
including garbage weeks such as 90-19700101. TTL deletes those rows on the
destination anyway, and attaching them recreates the destination's
block_numbers znode, which prune-partitions then forgets again. On
ClickHouse 25.8 an attach into a partition whose znode was forgotten can
also fail with ZNONODE when the replica still has the path cached.

Skip partitions that prune-partitions would classify as dead_past: past
retention plus --retention-margin-days (default 14). Both commands share
classify_partition, so they agree on what is dead. The filter only applies
when the source table is partitioned by (retention_days, toMonday(...)),
since other keys' IDs can't be read as a retention and a week, and the
partition key is only read when some candidate looks expired.
--include-expired restores the old behaviour. A single --partition-id that
is past retention is refused unless --include-expired is passed.

Co-Authored-By: Claude <[email protected]>
@onewland
onewland marked this pull request as ready for review October 19, 2026 22:13
@onewland
onewland requested a review from a team as a code owner October 19, 2026 22:13
@onewland
onewland merged commit d4d7df2 into master Oct 19, 2026
69 checks passed
@onewland
onewland deleted the oliver/attach-skip-expired-partitions branch October 19, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants