Cloud MOW: query-visible tablet cache can hold an incomplete delete bitmap after a full-compaction load, and later syncs never repair it

[asdf2014]

Search before asking

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

Version

master (line numbers below are as of e7b7f1d), cloud mode only, merge-on-write UNIQUE tables only.

What's Wrong?

On a cloud merge-on-write table, a plain SELECT can return rows that were already overwritten, so the same UNIQUE key appears more than once in the result. No query-side feature is involved; an ordinary scan is enough to hit it.

The wrong result is also not a brief window. It does not end when the compaction that caused it commits: in the reproduction below the query kept returning the resurrected rows after the compaction had finished, and only became correct once the tablet left that BE's cache. The persisted data is not damaged, though. The meta service reports the correct row count throughout, and restarting the BE (which drops the tablet cache) makes queries correct again, so this is a read-side defect confined to one BE's tablet cache.

A full-compaction submission loads a MOW tablet into the shared, query-visible tablet cache with sync_delete_bitmap=false:

• be/src/agent/task_worker_pool.cpp:1684: bool sync_delete_bitmap = compaction_type != CompactionType::FULL_COMPACTION; then engine.tablet_mgr().get_tablet(tablet_id, false, sync_delete_bitmap). Routine cumu/base pass true, only FULL passes false.
• be/src/cloud/cloud_compaction_action.cpp:153: same pattern for PARAM_COMPACTION_FULL on the HTTP action.

When that get_tablet is the cache-miss load that materializes the tablet, the cache now holds a CloudTablet whose rowsets are synced to some version V_ms while its delete bitmap is missing every marker up to V_ms. Two independent mechanisms make this state effectively permanent for readers:

1. The sync fast path ignores the flag. CloudTablet::sync_rowsets returns OK when _max_version >= options.query_version without consulting sync_delete_bitmap (be/src/cloud/cloud_tablet.cpp:317 and :338), so any later sync with query_version <= V_ms is a no-op and the short bitmap is served as is.
2. Even a real sync (query_version > V_ms) does not repair it. The delete-bitmap fetch requests full history (begin=0) only for the NEW rowsets of that sync; for OLD rowsets it requests only stamps in (old_max, new_max], where old_max = req.start_version() - 1 is the rowset watermark (be/src/cloud/cloud_meta_mgr.cpp:811, :1136-1141, :1144-1152). Markers stamped at or below V_ms that target pre-existing rowsets are outside every later fetch range.

Consequence: until the state is cleared, every MOW reader on that BE sees un-deduplicated data for keys whose delete markers fall in the missing range. A plain scan issues exactly this sync shape (be/src/exec/operator/olap_scan_operator.cpp:881-884, sync_delete_bitmap defaults to true in be/src/cloud/cloud_tablet.h:65) and then reads the tablet's bitmap as it stands, with no per-rowset re-fetch anywhere on the read path (be/src/storage/rowset_version_mgr.cpp:150 takes _tablet_meta->delete_bitmap_ptr() into the read source, which reaches the reader context at be/src/storage/tablet/tablet_reader.cpp:199). So overwritten or deleted rows resurface in query results. Readers also have no way to defend against or even detect the state: the tablet records no watermark for how far its delete bitmap is complete, so nothing distinguishes a fully synced bitmap from this truncated one.

Attribution: the bitmap-less load was introduced by #43337 (1216490), which added the sync_delete_bitmap parameter to CloudMetaMgr::sync_tablet_rowsets and CloudTabletMgr::get_tablet, threaded it into the cache-miss load_tablet path, and passed false for FULL on the HTTP compaction action. Before that commit a RUNNING merge-on-write tablet always had its bitmap synced on load. #62596 later mirrored the same pattern for the FE-pushed ADMIN COMPACT ... type='FULL' task, adding the second entry point. The intent of #43337 is sound on its own terms: full compaction recomputes the bitmap it needs (CloudFullCompaction::_cloud_full_compaction_update_delete_bitmap), so it genuinely does not need the synced one. The defect is that the skipping happens on the shared, query-visible cache load, so readers inherit the gap.

One scoping note: enable_compaction_rw_separation does not gate this. That flag assigns compaction work to the last-active load cluster (be/src/cloud/config.h:191-193) and is consulted only in background candidate selection (be/src/cloud/cloud_tablet_mgr.cpp:454 via CloudClusterInfo::should_skip_compaction); the two submission paths above never check it. A default single-cluster deployment is reachable.

What You Expected?

A MOW tablet resident in the query-visible tablet cache should never expose rowsets at version V without its delete bitmap being complete up to V. Equivalently, sync_rowsets with sync_delete_bitmap=true returning OK should guarantee bitmap completeness up to the requested version.

How to Reproduce?

Reproduced on a local cloud cluster (FoundationDB + MinIO + meta-service + cloud FE/BE) with a single BE, so the query necessarily lands on the BE that performed the load. On a multi-BE cluster a query routed to another BE looks correct, which can mask the problem.

The steps below use a debug point to make the window deterministic, so no race has to be won. The bug does not depend on it: the window is simply the span between the cache-miss load and the moment the tablet leaves the cache. The BE needs enable_debug_points = true.

1. Create a MOW table and give it rows to overwrite:

   CREATE TABLE repro.mow_bitmap (k INT, v VARCHAR(20)) UNIQUE KEY(k)
   DISTRIBUTED BY HASH(k) BUCKETS 1
   PROPERTIES ('replication_num'='1','enable_unique_key_merge_on_write'='true','disable_auto_compaction'='true');
   INSERT INTO repro.mow_bitmap VALUES (1,'a1'),(2,'a2'),(3,'a3'),(4,'a4'),(5,'a5');
   INSERT INTO repro.mow_bitmap VALUES (1,'b1'),(2,'b2'),(3,'b3');

   SELECT count(*) returns 5, as expected.
2. Restart the BE so the tablet is no longer resident in its tablet cache.
3. Block the compaction before it commits, then trigger a full compaction so its submission performs the cache-miss load:

   curl -X POST "http://<be>:8040/api/debug_point/add/CloudFullCompaction::modify_rowsets.block"
   curl -X POST "http://<be>:8040/api/compaction/run?tablet_id=<tablet_id>&compact_type=full"

4. Query the table. It returns 8 rows: 1|a1 and 1|b1 are both visible, likewise for k=2 and k=3. The overwritten rows have resurfaced, and the UNIQUE key is no longer unique in the result.
5. Remove the debug point and let the compaction commit. The BE logs finish CloudFullCompaction ... input_rows=8 output_rows=5 and the meta service reports num_rows: 5, yet the query still returns 8. Restarting the BE returns it to 5.

Anything Else?

Constraints any fix has to respect, from reading the surrounding code:

• Refilling DeleteBitmap is not enough: readers go through an aggregate cache with its own version watermark. A scan reads delete_bitmap->get_agg({rowset_id, seg_id, version}) (be/src/storage/rowset/beta_rowset_reader.cpp:184). On an exact key hit that returns the cached aggregate and never looks at the map (be/src/storage/tablet/tablet_meta.cpp:1876-1884); on a miss it starts from _get_rowset_cache_version(bmk) and ORs only map entries above that watermark (:1889-1891, with enable_mow_get_agg_by_cache defaulting to true). Markers recovered by a repair are by definition at or below the watermark, so a repair that only merges into the map can be invisible to every reader. DeleteBitmap::merge performs no invalidation (:1783-1797); the only invalidator is remove_rowset_cache_version (:1833), and all of its callers are rowset-removal paths. Compaction makes this deterministic rather than incidental: prefill_dbm_agg_cache_after_compaction (be/src/storage/tablet/base_tablet.cpp:2323, called from all three cloud compactions, enable_prefill_all_dbm_agg_cache_after_compaction defaults to true) populates the cache and the watermark from whatever bitmap the tablet holds at that moment. A fix has to invalidate the aggregate cache and the watermark for the rowsets it refills.
• A fix cannot live in CloudTablet::sync_rowsets alone. Callers reaching CloudMetaMgr::sync_tablet_rowsets directly (be/src/cloud/cloud_meta_mgr.cpp:583-587) bypass anything implemented there. That includes CloudTablet::calc_delete_bitmap_for_compaction (be/src/cloud/cloud_tablet.cpp:1349, :1396), the consumer whose damage is permanent: it derives the output rowset's delete bitmap from tablet_meta()->delete_bitmap() and commits it. Base and cumulative compaction can also skip their prepare_compact sync entirely through the need_sync_tablet heuristic (be/src/cloud/cloud_base_compaction.cpp:63-77), and the publish-side delete-bitmap task can skip its sync through should_sync_rowsets() (be/src/cloud/cloud_engine_calc_delete_bitmap_task.cpp:169-201). sync_tablet_rowsets_unlocked is the common point, and even there those two skip-the-sync paths need separate handling.
• Refetching everything from version 0 is too expensive to be the answer, and can fail permanently. A full_sync requests the complete bitmap history of every rowset (be/src/cloud/cloud_meta_mgr.cpp:1136-1141) and bypasses the local bitmap cache shortcut (:1119). On a large tablet that can exceed the meta service's max_get_delete_bitmap_byte (1 GB, cloud/src/common/config.h:340); retry_rpc does not retry that error class (be/src/cloud/cloud_meta_mgr.cpp:525-527) and enable_batch_get_delete_bitmap defaults to false, so the request is unchunked. If such a refetch is also what clears the "needs repair" state, a tablet that fails it stays marked and every later query repeats the same doomed fetch, becoming unqueryable until it is evicted or the BE restarts, which is worse than the wrong rows it was meant to fix. The cost has to be proportional to the actual gap, not to the tablet's whole history.
• The bitmap fetch has to be independent of whether new rowsets arrived. CloudMetaMgr::sync_tablet_delete_bitmap returns early on rs_metas.empty() (be/src/cloud/cloud_meta_mgr.cpp:1098), so on a tablet whose rowsets are already current an ordinary sync fetches no bitmap at all.

Fix directions:

1. Track a per-tablet delete-bitmap watermark (the version up to which the bitmap is known complete) as first-class tablet state, decided inside sync_tablet_rowsets_unlocked rather than in one caller. A consumer needing version V while the watermark sits at W fetches only (W, V], so the cost is proportional to the real gap and the 1 GB cliff is not in play; ordinary incremental syncs advance the watermark rather than leaving a sticky flag; and the refill invalidates the aggregate cache and _rowset_cache_version for the rowsets it touches, per the first constraint above. This looks like the cleanest direction.
2. Keep sync_delete_bitmap=false loads out of the query-visible cache: load a private tablet object for the compaction submission, or force a bitmap sync before publishing the tablet into the shared cache. Simpler to reason about, at the cost of either an extra tablet object or the sync [fix](cloud-mow)Full compaction no need to sync delete bitmap #43337 removed.

Two directions that look natural but do not work:

• A boolean "bitmap may be incomplete" flag plus an on-demand full_sync repair, decided in CloudTablet::sync_rowsets. It runs into all four constraints above: the aggregate cache can make the refill invisible, the direct CloudMetaMgr::sync_tablet_rowsets callers bypass it, and the from-version-0 refetch is both disproportionate and able to wedge a tablet permanently when it fails. Restricting it to a version-pinned reader (query_version > 0) to spare compaction the cost also excludes base and cumulative compaction, which read the bitmap and commit what they derive from it, leaving the consumer with the worst failure mode unprotected.
• Making CloudFullCompaction::prepare_compact skip the bitmap so the repair never lands on it. This is wrong for merge-on-write tables with cluster keys: their compaction reads with _direct_next_block and performs no key aggregation (be/src/storage/iterator/vertical_block_reader.cpp:247-253), so tablet_meta()->delete_bitmap(), passed in at be/src/storage/merger.cpp:285-287, is the only thing removing dead rows. Skipping it persists them into the compacted output.

Also worth stating plainly: adding a bitmap-coverage check to the sync fast path alone is not sufficient. It removes mechanism 1, but mechanism 2 (the old-rowset fetch keyed off the rowset watermark) still leaves the pre-watermark markers missing after a real sync.

There is a related but distinct narrow race in the same area: a stale-rowset GC sweep re-stamping a marker between a reader capturing rowsets and scanning the bitmap, the same exposure any MOW snapshot read faces. It has a different mechanism and probably a different fix (atomic capture plus scan, or a GC guard), so I kept it out of this issue.

Are you willing to submit PR?

• Yes I am willing to submit a PR!