Skip to content

[Bug] (MOW) Deleted key can be resurrected after base compaction prunes its delete-sign tombstone when a lower-sequence out-of-order write arrives #70206

Open

Description

@RoanHeNaN

Search before asking

  • I had searched in the issues and found no similar issues.

Description

Note on confidence: This report and the proposed fix are derived from reading the BE source code, not yet confirmed by a running regression test, and the proposed patch has not been compiled. It is filed as a proposal / needs-verification so maintainers can validate the root cause and discuss the fix direction. Code references are against master (around commit 173a0e2); line numbers are approximate.

Summary

For a Merge-On-Write (MOW) Unique Key table with a sequence column, a deleted key can be resurrected after base/full compaction physically prunes its delete-sign tombstone, if a later out-of-order write with a lower sequence value then arrives. This is a correctness bug on a single copy (including cloud/compute-storage-separation mode), and additionally causes permanent divergence between replicas in the local (compute-storage-coupled) mode because replicas compact independently.

Background

In a MOW table with a sequence column, rows with the same key are resolved by sequence value: the larger sequence wins, independent of version/arrival order. A delete (delete sign) with sequence S is semantically expected to permanently suppress any later write of the same key whose sequence < S.

A delete-sign row (tombstone) is a real stored row. Base compaction physically prunes these tombstones to reclaim space (enable_prune_delete_sign_when_base_compaction, default true).

Reproduction scenario (reasoned from code)

MOW table with a sequence column:

  1. Write a delete for key K1 with sequence = 1000.
  2. Base compaction runs and physically prunes the tombstone.
  3. A late write for K1 arrives with sequence = 900 (lower than the deleted 1000).
  • Expected: K1 stays deleted (because 900 < 1000, the delete should suppress it).
  • Actual: K1 is resurrected and becomes visible.

Impact

  • Correctness: Even a single copy (including cloud mode with no replicas) returns deleted data again.
  • Replica consistency (local mode): Replicas compact independently. A replica that has pruned the tombstone resurrects K1; a replica that has not still suppresses it. The two replicas return different results permanently and do not self-heal, because the delete bitmap is computed against physical data at write time.

Root cause

The MOW delete bitmap is computed at write time against the physical data present at that moment. The resurrection chain:

Step Code location Behavior
① Base compaction prunes the tombstone unconditionally be/src/storage/row_ttl.cpp build_row_visibility_filter (filters every delete_sign != 0 row); gated by _delete_sign_available in be/src/storage/tablet/tablet_reader.cpp (_init_delete_condition) The tombstone (carrying seq=1000) is physically removed. It does not check whether the table has a sequence column.
② Late write looks up the historical key be/src/storage/tablet/base_tablet.cpp lookup_row_key K1 not found → KEY_NOT_FOUND.
③ No sequence to compare against delete bitmap calculation Since the key is not found, the new seq=900 row is not marked in the delete bitmap → visible. Resurrection complete.

Contrast: if the tombstone were still alive (not yet compacted), lookup_row_key would hit it, return seq=1000, compare 900 < 1000, and mark the new row as deleted. So the sole root cause is that the tombstone — and with it the seq=1000 suppression watermark — is pruned too early.

Why only sequence-column tables are affected: without a sequence column, conflicts are resolved by version order; a later write always has a higher version and should win, so pruning the tombstone and making the new write visible is correct. No resurrection is possible.

Proposed fix: grace watermark

Do not change any delete/query/compaction resolution logic — only defer the physical reclamation of the tombstone until we can be confident no earlier (lower-sequence) write can still arrive. This mirrors Cassandra's gc_grace_seconds and Elasticsearch's index.gc_deletes.

Decision logic, added in _init_delete_condition for tables with a sequence column only:

grace = config.mow_sequence_delete_sign_grace_seconds
if grace < 0:    keep tombstones (never prune) for sequence-column tables
if grace == 0:   legacy behavior, prune immediately (default, no behavior change)
if grace > 0:
    newest = max(creation_time of input rowsets of this compaction)
    if now - newest <= grace:  do not prune this round (data still too new)
    else:                      allow pruning

Intuition: if even the newest input rowset is already older than grace, then any late write whose lateness is bounded by grace has already arrived and been applied, so pruning the tombstone is safe.

Why local wall-clock + creation_time is tolerant of replica clock skew: as long as grace >> (max out-of-order lateness + clock skew), by the time any replica prunes, the grace window has passed and no earlier write can arrive to cause resurrection or divergence. The grace window absorbs the per-replica timing difference.

Config

Config Default Meaning
mow_sequence_delete_sign_grace_seconds (BE, mutable, int64) 0 >0 keep tombstones at least this long (recommend ≥ max expected out-of-order lateness of the sequence column); 0 prune immediately (current behavior); <0 never prune

Default 0 means no behavior change for existing deployments; opt-in only.

Scope

  • Local and cloud covered by one change: both paths funnel through Merger::vmerge_rowsets (be/src/storage/merger.cpp) → TabletReader::_init_delete_condition, where tablet_schema and rs_splits are always populated.
  • Sequence-column tables only: non-sequence MOW tables are unaffected (zero behavior change, zero extra space).

Why the change is small

Key insight: as long as the tombstone is physically alive, the existing lookup_row_key + sequence comparison already produce the correct result. So the fix only needs to "keep the tombstone a little longer" — the read path, delete-bitmap calculation, and queries are untouched. The gate point also has direct access to the input rowsets and schema, so no plumbing through the compaction class is needed.

Change surface (proposed, not yet compiled/tested):

  • be/src/common/config.h / config.cpp: new config mow_sequence_delete_sign_grace_seconds + docs.
  • be/src/storage/tablet/tablet_reader.cpp: grace gate inside _init_delete_condition (~40 lines) + 2 includes.

Total ~68 lines, additive only; no existing logic removed. It is fail-safe: even an aggressive config only keeps tombstones longer (space cost), it never changes a correctness decision.

Assumptions / limitations

  1. The sequence column must be roughly correlated with wall-clock time (e.g. a CDC event time). The grace window is a time window; it is only meaningful when the out-of-order lateness has a time bound.
  2. If the sequence is unrelated to time (e.g. an arbitrary version number), the time window is meaningless; use grace < 0 (never prune) as a fallback.
  3. Cumulative compaction does not apply delete sign by default and is unaffected; if enable_delete_when_cumu_compaction is enabled, the same grace logic should be applied there too (follow-up).

Alternatives considered

  • Persist a per-key max-deleted-sequence watermark and consult it on KEY_NOT_FOUND: unbounded per-key state, requires read-path changes, and the side structure needs its own reclamation policy (grace again). Higher complexity, only saves space.
  • Row-level grace (decide per winning row by its source rowset's creation_time): needs to propagate source time to the filter; the coarse rowset-level approach is already correct and only costs some extra space.
  • Never prune (enable_prune_delete_sign_when_base_compaction=false / grace<0): correct but unbounded space growth; kept as the fallback for arbitrary sequences, not as default.

Test plan

  1. Single-copy repro: write K1 delete seq=1000 → trigger base compaction (prune tombstone) → write K1 seq=900 → assert K1 resurrects with grace=0 (exposes the bug) and stays deleted with grace>0.
  2. Replica-consistency case (local, 2 replicas) with different compaction timing → assert consistent query results after enabling grace.
  3. Boundary cases: creation_time right at the grace edge; sequence out-of-order crossing the edge.
  4. Regression protection: reuse enable_mow_compaction_correctness_check; assert bit-for-bit identical behavior when grace=0.
  5. Non-sequence tables: assert behavior identical to before.

Open questions for maintainers

  • Is the root-cause analysis correct, in particular the lookup_row_key → KEY_NOT_FOUND → resurrection path?
  • Preferred fix location/direction: grace watermark (this proposal) vs. persisting a sequence watermark?
  • Should the default eventually be a conservative positive value ("fix out of the box") rather than 0?
  • Should this be a table property rather than a BE config?

I'm happy to provide the draft patch and regression cases if the direction is agreed.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

Activity

  1. Doris-Breakwater commented on Nov 4, 2026

    @Doris-Breakwater
    Collaborator

    Breakwater-GitHub-Analysis-Slot: slot_cb8c6ccd6051

    Initial assessment: This is a credible MOW sequence/tombstone correctness report that warrants a deterministic regression test. The described lookup/bitmap mechanism is supported by source inspection, but the issue supplies no executed reproduction, so resurrection and replica divergence should remain needs verification, rather than being described as an already reproduced regression. The issue is currently open with no labels. Suggested triage area: BE storage / MOW / compaction / correctness.

    I inspected the configured local Doris repository at the exact cited commit, 173a0e2064e529bfa6f017608c40ff003c2f1066, without changing its branch or source. No build, running-cluster reproduction, or proposed-patch validation was performed.

    What the source establishes

    1. Base/full compaction enables delete-sign filtering when enable_prune_delete_sign_when_base_compaction=true; this decision has no sequence-column guard. The row filter then removes rows whose delete sign is nonzero. This supports loss of the tombstone's sequence information from the compacted output for the ordinary unique-key merge path. See the reader gate and the filter.
    2. When a matching stored key exists, the segment lookup compares sequence values and returns KEY_ALREADY_EXISTS for a lower incoming sequence. For a full-row upsert, bitmap calculation marks that incoming row deleted. When lookup instead returns KEY_NOT_FOUND, bitmap calculation continues without marking that row as a conflict. Thus, if compaction has removed the last relevant stored key/sequence and no other conflict exists, the proposed 1000 delete → compaction → 900 upsert chain follows from the code. See sequence comparison, rowset lookup, and bitmap handling.
    3. The shared-reader fix location is plausible for local and cloud execution, but the claim that everything funnels through Merger::vmerge_rowsets needs correction: compaction selects either horizontal or vertical merging. Both use TabletReader::init, and both block readers apply the visibility filter. Cloud compaction calls the shared merge_input_rowsets. See merge selection, cloud execution, and vertical filtering. This does not establish identical behavior for every schema variant, such as cluster keys or sequence mapping.

    Corrections and limits of the proposed fix

    • At this revision, enable_delete_when_cumu_compaction enables delete predicates, not delete-sign pruning. _delete_sign_available remains false for cumulative compaction even when that option is true; the source explicitly documents this distinction. The proposed cumulative-compaction follow-up is therefore not required on that premise. See the same reader gate.
    • A positive grace period is a conditional retention policy, not an unconditional sequence watermark. Once it expires, a sufficiently late lower-sequence replay can still resurrect the key. Safety requires an enforceable bound covering ingestion delays, retries, replay/backfill, and clock error. Sequence/time correlation alone does not prove that all earlier writes have been applied. Default 0 preserves the reported exposure.
    • Rowset creation_time is storage lifecycle metadata: local publication resets it, and rowset building also sets it to the current time. It is not the original event timestamp or a persisted per-key deletion watermark. Repeated compaction can refresh the age of retained tombstones, so the coarse policy can retain them much longer than the configured interval. Validate its local/cloud lifecycle semantics explicitly. See publication and rowset building.
    • If the intended scope is MOW+sequence only, the guard must check both conditions. Preserve checksum/query filtering semantics, and review other physical reclamation modes separately. A mutable BE-wide setting also needs consistent deployment across replicas; changing it cannot recover tombstones already discarded or repair existing divergence.
    • The existing pruning switch is explicitly annotated “only for test” in config.cpp. It is useful as a controlled diagnostic comparison, but should not be presented as a supported production workaround without maintainer confirmation.

    Missing evidence and recommended next step

    Please provide a runnable SQL/load regression with exact FE/BE build hashes, SHOW CREATE TABLE, sequence mapping/type, the delete ingestion method and payload, load labels/transaction IDs, and relevant compaction settings. An explicit delete-sign load and a predicate-based SQL DELETE should not be assumed equivalent without checking the actual generated sequence.

    Start with one replica and one key: optionally insert an older live row, load the seq=1000 tombstone, verify the key is logically absent, run base/full compaction to successful completion, verify that its selected inputs included the tombstone and that the output no longer contains it, then load the full-row seq=900 upsert. Supply normal query results and diagnostic hidden-column/rowset/bitmap evidence before and after each stage, plus the corresponding compaction log. Add unrelated rowsets if necessary to satisfy compaction selection; merely submitting a compaction request does not prove pruning occurred.

    Run an identical control with tombstone pruning disabled. Also compare a lower-sequence write arriving before compaction with one arriving after it. For the replica claim, hold all replicas at the same published version, deliberately vary pruning timing, and capture each replica's result and bitmap state before and after subsequent compaction. The source supports a risk of persistent divergence; it does not by itself prove that divergence is permanent under all repair or later-write paths.

    Before accepting a grace implementation, cover inside/outside-window arrivals, the exact boundary, negative/zero settings, higher/equal sequence values, repeated compaction, horizontal/vertical paths, local/cloud modes, and non-sequence/MOR controls. Use query-result assertions: the existing enable_mow_compaction_correctness_check_core / _fail checks concern compaction bitmap/row accounting and do not prove future late-write semantics. Maintainers should first decide the intended suppression lifetime; a bounded grace policy cannot satisfy the report's stated permanent suppression contract for arbitrary future replays.

  2. RoanHeNaN commented on Nov 4, 2026

    @RoanHeNaN
    ContributorAuthor

    Similar to #69409

  3. RoanHeNaN commented on Nov 4, 2026

    @RoanHeNaN
    ContributorAuthor

    Update: reproduced and fix verified on a local build

    The root-cause analysis above is no longer just code-derived — it is now
    confirmed on a local single-node cluster built with the proposed change.

    A regression test reproduces the resurrection with the legacy behavior and
    verifies suppression with the grace watermark:

    grace=0    -> k=1 visible   (resurrected -> reproduces the bug)
    grace=3600 -> k=1 deleted   (suppressed  -> the fix)
    grace=-1   -> k=1 deleted   (never prune -> fallback)
    Test 1 suites, failed 0 suites
    

    Scenario (MOW table with a sequence column):

    1. delete k=1 with a high sequence (seq=1000) via delete sign
    2. full compaction prunes the delete-sign tombstone
    3. late write for k=1 with a lower sequence (seq=900)

    With grace=0 (current behavior) k=1 is resurrected; with the grace window the
    tombstone is retained and the late lower-seq write is correctly suppressed. This
    also confirms the key step of the analysis — while the tombstone is alive,
    lookup_row_key + the sequence comparison already suppress the late write.

    Draft PR with the fix + regression test: #70217

    Still open for maintainer input on the direction (grace watermark vs. persisting a
    per-key sequence watermark) and on whether this should be a table property rather
    than a BE config.

  4. RoanHeNaN commented on Nov 5, 2026

    @RoanHeNaN
    ContributorAuthor

    Note on the current (coarse) grace implementation and a design question

    The draft PR #70217 implements the grace watermark at rowset granularity: in
    _init_delete_condition, delete-sign pruning for the whole compaction is deferred
    until the newest input rowset is older than the grace window.

    Known limitation (coarse gate). Because the decision is per-compaction (not
    per-row), under sustained ingestion a base compaction's input may always contain a
    recent rowset, so old tombstones that are already safe to prune get held back too,
    and delete-sign rows can accumulate in the base rowset. This is delayed
    reclamation, not a permanent leak (an ingestion lull lets the next compaction prune
    them) and it is tunable via the grace value. It only grows the delete-sign row
    count, not the delete bitmap — compaction still runs and still reclaims
    delete-bitmap entries for covered rows as usual.

    Why not row-level grace yet. Keeping only the tombstones actually within the
    window (decided per winning row by its source-rowset age) would remove the
    accumulation, but:

    • it is a space optimization, not a correctness change — the coarse gate already
      fixes the resurrection/divergence bug in full;
    • delete-sign filtering must run post-merge on the winning row, where per-row
      source-rowset provenance isn't available; threading it through the merge iterator
      → block reader → visibility filter is a cross-layer change on the hot path, and
      filtering pre-merge per rowset is incorrect (an older non-delete row in another
      input rowset would resurrect within the same compaction).

    Design question for maintainers. Before investing in the heavier version, which
    direction is preferred:

    1. keep the grace-watermark approach and add row-level grace as a follow-up, or
    2. instead persist a per-key "max deleted sequence" watermark that the
      KEY_NOT_FOUND path can consult, so tombstones can be pruned immediately?

    Happy to go either way based on your input.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/storageIssues or PRs related to storage engine

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions