Doris-Breakwater commented on issue #69409: URL: https://github.com/apache/doris/issues/69409#issuecomment-6321061083
Breakwater-GitHub-Analysis-Slot: slot_d814bb63ac39 The report warrants **storage/compaction correctness triage**: source inspection supports the proposed loss of the deleted key's sequence boundary. The important distinction is that compaction need not change the immediate SELECT result; it changes which *subsequent writes* are accepted for that key. **Evidence and confidence** - **Reported runtime behavior:** the supplied reproduction demonstrates resurrection after base/full compaction for MOW and MOR, on master `15eb3f4e432b1d4a39f89457956135da80b90940` and 4.1.5 build `917b20be8`. SQL DELETE and stream-load DELETE are also reported affected. I have not independently executed these reproductions. - **Verified in source:** I inspected the exact master commit and `917b20be83878e98f94e3ebf7efccc80346e3e50`, which the local `4.1.5-release` tag resolves to. Both contain the relevant pruning and sequence-comparison paths described below. The issue currently has no labels; suggested classification is Bug / storage / compaction, with elevated priority because older data can become visible after a delete. **Verified code mechanism** 1. At the reported master commit, [`TabletReader::_init_delete_condition`](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/tablet/tablet_reader.cpp#L444-L470) enables delete-sign pruning for base and full compaction when `enable_prune_delete_sign_when_base_compaction` is true, without checking for a sequence column. [`VerticalBlockReader`](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/iterator/vertical_block_reader.cpp#L530-L583) and [`BlockReader`](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/iterator/block_reader.cpp#L850-L893) then filter the winning delete-sign row out of the compaction output. 2. For MOW, [`Segment::lookup_row_key`](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/segment/segment.cpp#L1325-L1341) returns `KEY_ALREADY_EXISTS` when the incoming sequence is smaller than the stored sequence. [`BaseTablet::calc_segment_delete_bitmap`](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/tablet/base_tablet.cpp#L794-L837) marks that incoming row deleted; `KEY_NOT_FOUND` instead skips this conflict handling. Once compaction removes the last row for key 1, the lookup has no sequence-900 entry against which to reject sequence 500. This explains why a successful/visible load can be ignored before compaction and become query-visible afterward. 3. For MOR, the [merge comparator](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/storage/iterator/vcollect_iterator.cpp#L218-L245) prioritizes the larger sequence for equal keys. Removing the winning tombstone also removes the comparison candidate needed to suppress a later, smaller-sequence row. 4. The 4.1.5 build has the [same base/full pruning condition](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/tablet/tablet_reader.cpp#L499-L526). This is consistent with the cross-version report, rather than evidence of a master-only regression. The mechanism is therefore strongly supported by static inspection. Independent runtime confirmation, the complete scope across other compaction paths, and a validated fix remain outstanding. [#17739](https://github.com/apache/doris/issues/17739) supports the expected suppression of older writes while the delete record exists; it does not establish a tombstone garbage-collection boundary. [#51874](https://github.com/apache/doris/pull/51874) extended pruning to full compaction. [#59441](https://github.com/apache/doris/pull/59441) is closed and unmerged, so it should not be treated as an available fix. **Remaining verification and next steps** 1. Run the supplied script on fresh tables with pruning enabled versus `enable_prune_delete_sign_when_base_compaction=false`, set **before** the delete/compaction, for MOW/MOR and base/full compaction. Record the effective BE setting, actual successful compaction status and input/output version ranges, physical tombstone presence, and the final normal SELECT. The expected distinguishing result is that retaining the sequence-900 tombstone continues to suppress sequence 500. The report already provides enough version, DDL, and sequencing information to begin reproduction; logs around publish/compaction are useful if this control behaves differently. 2. Treat that setting as a diagnostic control: the [configuration source explicitly marks it “only for test”](https://github.com/apache/doris/blob/15eb3f4e432b1d4a39f89457956135da80b90940/be/src/common/config.cpp#L1985-L1987). Its production use needs maintainer validation and a tombstone storage-cost assessment. Changing it after tombstones have been pruned cannot recover their sequence values. 3. Evaluate preserving the winning tombstone for sequence-enabled UNIQUE KEY compaction, or storing an equivalent persistent per-key sequence boundary. A time delay alone cannot guarantee rejection of arbitrarily late older writes without an explicit lateness bound. Keep query/checksum deletion semantics correct: `_delete_sign_available` also serves checksum and cold-data readers, so a blanket change to this shared flag needs separate review. 4. Add regression coverage for MOW/MOR, base/full, both compaction reader implementations, repeated compactions, and an all-rows-deleted output. Cover explicit delete-sign writes, stream-load DELETE, and SQL DELETE on MOW. Verify that smaller sequences remain suppressed and a larger sequence can reinsert; test equal-sequence semantics separately. Preserve existing behavior for tables without sequence columns. The existing `test_config_prune_delete_sign.groovy` covers a MOW table without a sequence column and does not exercise the late-write boundary here. No code changes were made during this investigation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
