[core] Wire sorted primary-key indexes into writes#8602
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
I found two blocking correctness issues in the mixed/multi-index write lifecycle, plus a reproducible failure in the new concurrency test.
| vectorFactory == null | ||
| ? null | ||
| : vectorFactory.create( | ||
| partition, bucket, dataFiles, payloads, executor); |
There was a problem hiding this comment.
restoredPayloads is the full result of scanSourceIndexes, so it contains the payloads for every configured source-backed definition. Passing that unfiltered list into the vector factory makes PkVectorBucketIndexState validate BTree/Bitmap payloads as vector segments and throw Vector payload ... has a different index type. A table configured with both a vector index and a scalar index therefore cannot restore a bucket writer after scalar payloads have been committed. Please filter the restored payloads by the vector field/index type before creating the vector maintainer, and add a mixed vector + scalar write/restore test.
| } | ||
|
|
||
| if (waitCompaction) { | ||
| prepareSortedBlocking(appendIncrement, compactIncrement); |
There was a problem hiding this comment.
The coordinator publishes each maintainer's state and merges its files into the caller increments before later maintainers have succeeded, but there is no coordinator-level rollback. If an earlier vector/scalar definition completes and a later blocking scalar prepareCommit is interrupted, the overall call throws while the earlier definition remains covered and its generated payload remains in the increment. Retrying or discarding that failed prepare can then omit the payload or leave it orphaned. Individual maintainers only roll back their own invocation. Please make this operation atomic across all definitions (or defer publication until all definitions succeed) and cover an interruption after the first of two scalar definitions completes.
| assertThat(firstCompleted.await(5, TimeUnit.SECONDS)).isTrue(); | ||
| maintainer.prepareCommit( | ||
| DataIncrement.emptyIncrement(), CompactIncrement.emptyIncrement(), false); | ||
| assertThat(secondStarted.await(5, TimeUnit.SECONDS)).isTrue(); |
There was a problem hiding this comment.
This test is racy and fails consistently on my machine. firstCompleted is counted down inside the callable immediately before it returns, so the following non-blocking prepareCommit may still observe Future.isDone() == false and return without starting the second definition; the await here then times out. Please wait/poll until the first future is actually consumed (for example, retry non-blocking prepare until the second build starts) instead of treating the in-callable latch as future completion.
b4dd08a to
4520ae2
Compare
4520ae2 to
ca917ff
Compare
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head. The mixed vector/scalar restore filtering, coordinator-wide rollback, and scalar build concurrency regression are addressed. The focused core test suite and repeated concurrency test pass locally.
Purpose
Wire source-backed BTree and Bitmap indexes into the primary-key table write lifecycle.
Changes
IOManager.indexTypeand field ID at each maintainer boundary, so scalar payloads never enter vector state.-2; the fixed-bucket runtime view used by background compaction creates and publishes the coordinator.Impact
Compacted primary-key table files now automatically receive configured BTree and Bitmap payloads. Read-side source pruning remains intentionally deferred to a follow-up PR.
Validation
mvn -pl paimon-codegen-loader -am -Pfast-build -DskipTests packagemvn -pl paimon-core -am -Pfast-build -DfailIfNoTests=false -DwildcardSuites=none -Dtest=BucketedVectorIndexMaintainerTest,PkVectorBucketIndexStateTest,BucketedSortedIndexMaintainerTest,BucketedPrimaryKeyIndexMaintainerTest,PrimaryKeySortedIndexMaintenanceTest,PrimaryKeyIndexWriteTest,PrimaryKeyVectorIndexWriteTest,BtreeGlobalIndexTableTest test(46 tests)mvn -pl paimon-core -DskipTests validate