Skip to content

[hive] Clean up done markers in Hive partition deletion - #10964

Merged
JingsongLi merged 5 commits into
apache:masterfrom
kerwin-zk:fix-expire-done-partition-cast
Nov 4, 2026
Merged

JingsongLi merged 5 commits into
apache:masterfrom
kerwin-zk:fix-expire-done-partition-cast

Conversation

@kerwin-zk

@kerwin-zk kerwin-zk commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Partition expiration on a Hive-backed table can commit the deletion of an ordinary INT partition and then fail when the synthetic 20230101.done marker is passed to typed partition deletion or branch scans. Both normal and chain-table expiration are affected.

Keep marker cleanup inside HiveCatalog.dropPartitions. After the requested data partitions are deleted, HiveCatalog derives the corresponding markers from those original specs, appending .done to the last partition key in schema order, and removes only their HMS entries. Marker cleanup does not truncate Paimon data or create another snapshot. Normal and chain-table expiration call the ordinary partition-drop path once per batch. No new public methods are added to Catalog or PartitionModification, and delegate/caching catalogs need no changes.

HMS partition names are shared across branches. Preserve a marker if any branch still owns its underlying ordinary partition, or if any branch, including the current branch, owns a real partition with the marker's literal spec. These checks compare rendered partition specs from the latest physical branch metadata using each branch's schema, without fallback reads or parsing synthetic values. Ownership scans use the storage scan directly, independently of persisted query startup options such as snapshot IDs or tags; query settings remain unchanged. This enumerates broader partition metadata than a typed predicate; ordinary deletion retains its existing filtered branch check.

Marker identity comes from the original deletion request. An explicit DROP of a real STRING partition ending in .done still deletes its data, retains the usual branch protection for its HMS entry, and cleans up its own .done.done marker when no branch needs it.

Tests

  • INT expiration with/without actual markers and with empty branches; shared ordinary partitions and markers remain until their last owning branch expires them.
  • TTL name collisions with another branch's real STRING .done partition, and ordinary deletion with a real colliding partition on the current branch.
  • Explicit deletion of real STRING .done data and its own marker, including shared branch ownership.
  • INT chain-table expiration with/without markers, default/batch-size-one cleanup, snapshot/delta branches, and retained anchors.
  • Direct deletion of a multi-key partition with a reversed input-map order, an INT final key, an empty branch, and a retained STRING value ending in .done.
  • Six historical-read regressions cover persisted snapshot/tag selectors, real marker-name collisions on the current/other branch, and ordinary partitions shared with another branch. Latest data and HMS registrations remain intact, configured historical queries keep their original behavior, and cleanup adds only the ordinary deletion snapshot. All six scenarios lose live HMS registrations before the ownership-scan fix.
  • Unexpired data remains and marker cleanup creates no additional data snapshot. Existing upstream TTL test inputs and assertions remain unchanged; fake metastore implementations model Hive's internal marker cleanup.

Local validation (JDK 17), with quality checks enabled:

mvn -B -nsu -pl paimon-hive/paimon-hive-catalog -am \
  -Dmaven.test.additionalClasspath=/path/to/hadoop-client-runtime-3.3.5.jar \
  -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false \
  -Dtest=PartitionExpireTest,ChainTablePartitionExpireTest,CachingCatalogTest,DelegateCatalogTest,HiveCatalogTest package

The extra classpath entry supplies the Hadoop client runtime corresponding to the client API on the local test classpath; no dependency changes are included.

All 195 tests in the five targeted classes pass with no skips. Standard packaging, Spotless, Checkstyle, and git diff --check pass.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requirement fit: SUPPORTED. Implementation: FINDINGS.

Separating synthetic marker cleanup from data truncation has clear end-to-end value. On ee50ad6, the normal JDK 8 reactor package passes all 161 targeted tests (PartitionExpireTest, CachingCatalogTest, DelegateCatalogTest and HiveCatalogTest), with quality checks enabled; upstream CI is also green. An additional actual-table branch case still reproduces the exception this PR intends to fix, as described below. Temporary reviewer changes were removed.

@JingsongLi
JingsongLi dismissed their stale review November 3, 2026 06:34

Verified fixed at 08b5cfb. Requirement fit: SUPPORTED; implementation: CLEAN on this reviewed head. All 168 targeted JDK 8 core/Hive tests pass with normal quality checks, including empty-branch INT expiration, shared-branch marker retention and real STRING .done partitions. The original P2 is resolved; withdrawing this blocking review. The new head's CI is still running, so this is not an approval or merge.

@kerwin-zk
kerwin-zk requested a review from JingsongLi November 3, 2026 08:01
@JingsongLi

Copy link
Copy Markdown
Contributor

Could we keep .done cleanup inside HiveCatalog.dropPartitions instead of adding new public methods to Catalog and PartitionModification? After deleting the requested data partitions, HiveCatalog could derive the corresponding marker specs from those original specs and remove only their HMS entries, without truncating data or creating another snapshot. Normal and chain-table expiration would then use the ordinary partition-drop path without a second call for markers. This would keep the Hive marker convention within HiveCatalog and avoid expanding the public API for this fix.

The cleanup needs to account for the shared HMS namespace: retain a marker if any branch still owns its underlying ordinary partition, or if any branch, including the current branch, contains a real partition whose literal spec equals the marker spec. These checks should use physical branch partitions without fallback reads and compare rendered partition specs, rather than converting synthetic values such as 20230101.done to the partition type. Marker identity should come from deriving it from the original deletion request, not from treating every .done suffix as synthetic; an explicit DROP of a real STRING partition ending in .done must still delete its data with the usual branch protection for HMS metadata.

@kerwin-zk kerwin-zk changed the title [core] Drop expired done partitions only from the metastore [hive] Clean up done markers in Hive partition deletion Nov 3, 2026
@kerwin-zk

Copy link
Copy Markdown
Contributor Author

Could we keep .done cleanup inside HiveCatalog.dropPartitions instead of adding new public methods to Catalog and PartitionModification? After deleting the requested data partitions, HiveCatalog could derive the corresponding marker specs from those original specs and remove only their HMS entries, without truncating data or creating another snapshot. Normal and chain-table expiration would then use the ordinary partition-drop path without a second call for markers. This would keep the Hive marker convention within HiveCatalog and avoid expanding the public API for this fix.

The cleanup needs to account for the shared HMS namespace: retain a marker if any branch still owns its underlying ordinary partition, or if any branch, including the current branch, contains a real partition whose literal spec equals the marker spec. These checks should use physical branch partitions without fallback reads and compare rendered partition specs, rather than converting synthetic values such as 20230101.done to the partition type. Marker identity should come from deriving it from the original deletion request, not from treating every .done suffix as synthetic; an explicit DROP of a real STRING partition ending in .done must still delete its data with the usual branch protection for HMS metadata.

@JingsongLi done

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requirement fit: SUPPORTED. Implementation: FINDINGS.

The previous synthetic INT/branch failure and public-API concern are addressed in this revision. Normal JDK 8 checks and all 189 targeted Hive/catalog/normal-TTL/chain-TTL tests pass. An additional real-table regression found one current-branch ownership error below. Its latest-data assertion passes before the HMS registration assertion fails; temporary reviewer source changes were removed.

@kerwin-zk
kerwin-zk requested a review from JingsongLi November 4, 2026 04:31
@JingsongLi
JingsongLi merged commit 7ad15d0 into apache:master Nov 4, 2026
18 checks passed
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.

2 participants