Skip to content

fix table clone visibility for repeated alter#24987

Open
gouhongshen wants to merge 5 commits into
matrixorigin:mainfrom
gouhongshen:codex/issue-24762-main
Open

fix table clone visibility for repeated alter#24987
gouhongshen wants to merge 5 commits into
matrixorigin:mainfrom
gouhongshen:codex/issue-24762-main

Conversation

@gouhongshen

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

fixes #24762

What this PR does / why we need it:

Repeated ALTER TABLE statements in one transaction can clone from a table or hidden index table that was produced by an earlier ALTER in the same transaction. The old TableMetaReader only read the committed partition snapshot and constructed the local reader without the current transaction write offset, so the second clone could miss uncommitted object stats from the first clone. The base table data remained visible, but secondary index data could be missing after commit.

This PR keeps the clone path and makes the clone source reader transaction-visible:

  • capture the TableMetaReader transaction write offset
  • include uncommitted data and tombstone object stats when collecting clone metadata
  • use NewLocalDataSource with the same txnOffset for appendable/in-memory clone reads
  • include row-level uncommitted tombstones when collecting tombstone rows
  • reuse a common helper for collecting uncommitted data/tombstone object stats

It also extends git4data clone BVT coverage with repeated ALTER cases, including a stress case with secondary, unique, fulltext, and ivfflat vector indexes plus multiple ALTER operations in one transaction.

Validation:

  • make build
  • clone_in_alter_table_2: 81/81 passed
  • git4data/clone: 1177/1177 passed
  • full git4data: 4403/4403 passed
  • git diff --check

@XuPeng-SH XuPeng-SH 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.

Found a rollback regression in the new clone visibility path.

In pkg/vm/engine/disttae/table_meta_reader.go:230-270, the refactor routes txn-local objects from an earlier ALTER through appendObjectStats, which still calls cloneTxnCache.AddSharedFile unconditionally. Those objects were created by the current txn, so marking them as shared makes GCObjsByIdxRange/gcFiles skip them on rollback for clone txns. As a result, ALTER ...; ALTER ...; ROLLBACK can leak the data/tombstone objects produced by the earlier ALTER. Before this change only committed source objects from pState reached that cache, so the rollback path was safe. Please keep committed/source objects shared, but do not register txn-local objects from collectUnCommittedDataObjs / collectUnCommittedTombstoneObjs as shared, and add rollback coverage for this unhappy path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working kind/test-ci size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 一些ddl操作之后,字符串列进行字符串比较失效。只能通过binary方式比较

4 participants