ClickHouse / ClickHouse/ClickHouse
Lightweight update silently skips acknowledged rows in a partition whose local parts were all empty at submission — partition pruning freezes the read bound before the update_sequential_consistency sync
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
**Describe what's wrong**
Lightweight updates consume the automatic mutation partition pruning (`optimize_mutations_with_partition_pruning`, default on, added in #110968) as a hard read bound, and they freeze that bound before the `update_sequential_consistency` sync runs. When a partition's parts on the initiating replica are all empty at block-allocation time, the partition is pruned out and recorded as analyzed (the same analysis-side hole as #117114), so it gets no block number and is absent from the update's `PartitionIdToMaxBlock` map. The part-picking code then skips every part of that partition — including the part that the update's own sequential-consistency sync fetches moments later. The `UPDATE` returns success, produces no patch rows for those rows, and acknowledged data in the matched partition silently keeps its pre-update values on every replica, at pure default settings.
This is the lightweight-update manifestation of the analysis hole reported in #117114, but through a different consumption seam, so a fix scoped to the mutation `block_numbers` path alone would not close it:
* `StorageReplicatedMergeTree::updateLightweight` (`src/Storages/StorageReplicatedMergeTree.cpp:8830`) calls `allocateBlockNumbersInAffectedPartitions` (`:8839`) — the pruning happens here, with the `PartitionPruner::canBePruned` `if (part.isEmpty()) return true;` short-circuit (`src/Storages/MergeTree/PartitionPruner.cpp:26`) ruling the all-empty partition out before its value is evaluated, and `getPartitionIdsPrunedByPredicate` recording it as analyzed, which suppresses the ZooKeeper widening.
* The allocated block numbers become the read bound: `context_copy->setPartitionIdToMaxBlock` (`:8848`). A partition absent from the map is skipped wholesale by part selection (`MergeTreeDataSelectExecutor.cpp`, the `max_block_numbers_to_read` check: absent partition → skip part).
* Only after the bound is frozen does `update_sequential_consistency = 1` (the default) run `waitForCommittingOpsToFinish` → `waitForProcessingQueue` (`:8859`), whose documented purpose is "set of parts is updated to the latest version before execution of update". The sync completes, the initiator fetches the remote part into the pruned partition, and the read then skips it because the bound predates the fetch.
So the sequential-consistency contract is violated by the pruning: the update pays the sync, receives the rows, and still does not update them.
**Does it reproduce on the most recent release?**
Reproduces on current master containing #110968 (merge commit `596fd32c14f`; verified on a release build of the merged branch head `5c75d9ba25e`). No release contains the feature yet.
**How to reproduce**
Two replicas `r1`, `r2` of one `ReplicatedMergeTree` table with the lightweight-update columns enabled; every query-level setting default except `enable_lightweight_update`. `SYSTEM STOP REPLICATION QUEUES` stands in for ordinary replication lag on the initiator at submission time; it is started again while the `UPDATE` is inside its own sequential-consistency wait, which is exactly the "sync heals the lag" path the setting promises.
```sql
-- on both replicas
CREATE TABLE t (p UInt8, x UInt64, v UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/tables/{database}/t', '{replica}')
PARTITION BY p ORDER BY x
SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1;
-- on r1
INSERT INTO t SELECT 1, number, 1 FROM numbers(50);
INSERT INTO t SELECT 2, number, 1 FROM numbers(50);
-- empty out partition 2; the resulting 0-row part stays Active until the cleanup thread fires
ALTER TABLE t DELETE WHERE p = 2 SETTINGS mutations_sync = 2;
-- r1 lags behind
SYSTEM STOP REPLICATION QUEUES t;
-- on r2: this insert is acknowledged to the client
INSERT INTO t SELECT 2, 1000000 + number, 1 FROM numbers(30);
-- on r1: the update under test. It allocates block numbers (partition 2 is pruned as all-empty),
-- then parks in the update_sequential_consistency queue sync.
UPDATE t SET v = 999 WHERE p = 2 SETTINGS enable_lightweight_update = 1;
-- on r1, from a second connection, while the UPDATE is waiting:
SYSTEM START REPLICATION QUEUES t;
-- the UPDATE's sync now completes: r1 fetches the 30-row part, the UPDATE reads, and returns success.
-- on either replica, after replication converges:
SELECT count() FROM t WHERE p = 2 AND v = 1;
```
Observed result: the `UPDATE` completes successfully, `system.parts` shows no patch part was created, and `SELECT count() FROM t WHERE p = 2 AND v = 1` returns `30` on both replicas — all 30 acknowledged rows in the matched partition kept the pre-update value.
Controls, identical schedule:
* `optimize_mutations_with_partition_pruning = 0` on the `UPDATE`: a 30-row patch part is created and the stale count is 0 on both replicas.
* Pruning on but no lag at submission (skip the `STOP REPLICATION QUEUES`, `SYSTEM SYNC REPLICA` before the `UPDATE`): partition 2 is non-empty at allocation, a 30-row patch part is created, stale count 0.
**Expected behavior**
The `UPDATE ... WHERE p = 2` updates every row of partition 2 that its sequential-consistency sync made visible: `SELECT count() FROM t WHERE p = 2 AND v = 1` returns 0 on all replicas.
**Additional context**
The same three analysis-side guard holes already reported for the mutation vehicle (#117113 `IN ` deferred sets, #117114 all-empty partitions, #117115 ALIAS-laundered nondeterminism) all feed lightweight updates through `getPartitionIdsAffectedByCommands`, since `updateLightweight` shares `allocateBlockNumbersInAffectedPartitions` with mutations. This report demonstrates the all-empty-partition hole because it needs no non-default settings at all, but a fix should cover the lightweight-update read-bound consumption of the pruned set as well as the mutation `block_numbers` consumption.
Contributor guide
Assessment
This issue has not been assessed yet.