Repository navigation
[core] Fix lookup returning rows deleted by deletion vectors - #10813
Stephen0421 wants to merge 1 commit into
Conversation
|
|
||
| private List<IndexManifestEntry> currentDeletionVectorEntries() { | ||
| Snapshot latest = snapshotManager.latestSnapshot(); | ||
| long snapshotId = latest == null ? -1L : latest.id(); |
There was a problem hiding this comment.
[P1] Keep deletion vectors aligned with the streamed file update
Loading deletion vectors from the latest table snapshot can make them newer than the beforeFiles / dataFiles being applied. If snapshot N adds a deletion vector for an old data file and snapshot N+1 rewrites that file and removes the vector, a lagging query operator processing N reads N+1's now-empty vector state while its lookup levels still contain the old file. The deleted row is then returned until the N+1 file update arrives. This is observable in QueryExecutorOperator, which applies updates individually while KvQueryServer serves concurrent requests. Please propagate the update's snapshot/index state with the file event, or otherwise update files and deletion vectors from the same snapshot.
There was a problem hiding this comment.
Fixed. refreshFiles now takes the snapshot id of the file event, and deletion vectors are loaded from that snapshot instead of latestSnapshot().
QueryExecutorOperator reads the snapshot id from field 0 of the FileMonitorTable row and passes it through. A lagging executor applying snapshot N therefore still sees N's deletion vectors, even if N+1 has already rewritten the file and dropped the vector. The deleted row is not returned.
If that snapshot has expired, refresh throws OutOfRangeException instead of serving the files with no deletion vectors. FileStoreLookupFunction already catches this and reopens from a live snapshot.
JingsongLi
left a comment
There was a problem hiding this comment.
The deleted-row lookup problem has clear end-to-end value. The 3 new DV tests and 2 existing non-DV lookup tests pass locally with normal build checks. An additional real-table regression exposes a newly introduced snapshot mismatch: PR head returns null, while its parent returns the expected S1 value for the same held file plan.
| } | ||
|
|
||
| private List<IndexManifestEntry> currentDeletionVectorEntries() { | ||
| Snapshot latest = snapshotManager.latestSnapshot(); |
There was a problem hiding this comment.
[P1] Pair deletion vectors with the snapshot of the supplied files
latestSnapshot() can be newer than the files being installed. I reproduced this using real writes/compaction: at S1, compact 20 keys including (1, 10) and retain its planned data files; write (1, 99) and partially compact S2; then call refreshFiles with the held S1 files. This code reads S2's vector, masks key 1's position in S1, and lookup returns null (key 2 remains visible). The same test against the parent implementation returns 10. Key 1 exists in both S1 and S2, so this is neither a valid stale value nor the current value.
This interleaving is reachable when QueryFileMonitor has planned/enqueued an S1 event and S2 commits before QueryExecutorOperator processes it; the query server continues serving lookups between events. A miss can silently drop a lookup-join match. Please carry the event/split snapshot ID (or matching DV metadata) through refreshFiles and load the corresponding deletion vectors, so file levels and visibility are installed from the same snapshot. Add a regression with a held file plan and an intervening update/compaction.
There was a problem hiding this comment.
Fixed. The event/split snapshot id is now passed into refreshFiles, and deletion vectors are loaded from that snapshot together with the files.
testDeletionVectorUsesTheFileSnapshot holds the S1 file plan, writes (1, 99), partially compacts S2, then refreshes with the held S1 files. Key 1 stays 10 and key 2 stays 20. The S2 vector is no longer applied to the S1 file.
PrimaryKeyPartialLookupTable passes split.snapshotId() for both DataSplit and IncrementalSplit. A negative id means the files are not committed yet, and no deletion vector is applied. If the snapshot file has expired, refresh throws OutOfRangeException rather than looking up without its deletion vectors.
Lookup ignored deletion vectors after compaction dropped the delete record, so partial lookup and the query service returned deleted rows. Deletion vectors are now loaded from the snapshot that produced the installed files. A newer commit can no longer hide or reveal rows in a file plan that has not been applied yet. If that snapshot has expired, refresh fails with OutOfRangeException instead of serving the files without their vectors.
74b0738 to
15606ce
Compare
Purpose
LocalTableQuerybuilds lookup files without applying deletion vectors, and checks only the row kind of the cached record. After compaction drops the delete record and marks the old position with a deletion vector, Flink partial lookup and the query service return the deleted row.Deletion-vector tables that use the deduplicate merge engine and have no sequence field now look up from level 0, so an uncompacted delete is visible. The cached entry stores the row position. If that position is contained in the bucket's current deletion vector, lookup returns null and does not continue into older levels. The vector is not applied while building the lookup file, because dropping the key there would fall through to a stale value.
Other merge engines and tables with a sequence field keep their previous start level. A level-0 record is not yet merged, so returning it would skip partial-update, aggregation, and sequence-field behavior. Those tables still honor deletion vectors on the files they do read.
Each
refreshFilesreloads the bucket deletion-vector index from the latest snapshot. Streaming plans do not carry deletion files, and compaction can mark a file that is absent from the current split. The parsed vectors are reused while the index files are unchanged, and the cached file list is recorded only after the read succeeds. Deletion-vector tables do not use remote lookup files, because those files do not store the row position.Tests
LocalTableQueryDeletionVectorTestPrimaryKeySimpleTableTest#testTableQueryForNormaland#testTableQueryForLookupfor the existing non-deletion-vector lookup path.mvn -pl paimon-core spotless:check