fix(clickhouse): quorum writes on replicated clusters to stop normalize dropping rows - #4676
Open
andreyzhelnin-st wants to merge 5 commits into
Open
fix(clickhouse): quorum writes on replicated clusters to stop normalize dropping rows#4676andreyzhelnin-st wants to merge 5 commits into
andreyzhelnin-st wants to merge 5 commits into
Conversation
…ze dropping rows On a multi-replica ReplicatedMergeTree cluster, normalize populates the raw table and then reads it back via INSERT ... SELECT. Those statements can run on different connections/replicas (e.g. behind a load-balanced host), so the read may hit a replica that has not yet replicated the just-written raw parts. select_sequential_consistency (already set on reads) only guarantees visibility of quorum-inserted blocks; with the default insert_quorum=0 it enforces nothing, so the normalize SELECT can return fewer/zero rows, mark the batch processed, advance the normalize pointer, and silently drop those rows permanently. Complete the read-your-writes contract on replicated clusters by writing with insert_quorum=auto and insert_quorum_parallel=0 (required for sequential consistency reads to be honored). Gated behind PEERDB_CLICKHOUSE_ENABLE_REPLICATED_QUORUM (default true). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On Replicated clusters we already write raw/normalize inserts with quorum (insert_quorum=auto, insert_quorum_parallel=0). Explicitly set select_sequential_consistency=1 in the same block so the read side of the read-your-writes contract stays paired with the quorum writes regardless of the global settings default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andreyzhelnin-st
requested a deployment
to
external-contributor
August 7, 2026 07:22 — with
GitHub Actions
Waiting
andreyzhelnin-st
requested a deployment
to
external-contributor
August 7, 2026 07:22 — with
GitHub Actions
Waiting
Contributor
Author
|
Could you please review this @jgao54 - it works good in our prod for a few days? |
Contributor
Author
|
@serprex could you please have a look? |
jgao54
reviewed
Aug 10, 2026
| "(insert_quorum=auto, insert_quorum_parallel=0) so that select_sequential_consistency reads " + | ||
| "see them regardless of which replica serves the read. Prevents normalize silently dropping " + | ||
| "rows when the read hits a replica that has not yet replicated the just-written raw parts.", | ||
| DefaultValue: "true", |
Contributor
There was a problem hiding this comment.
The default value should be false here to ensure backwards-compatibility (it's also the default setting in ClickHouse so would be good to keep it consistency)
jgao54
reviewed
Aug 10, 2026
| if quorum, err := internal.PeerDBClickHouseEnableReplicatedQuorum(ctx, env); err != nil { | ||
| return nil, fmt.Errorf("failed to load replicated quorum config: %w", err) | ||
| } else if quorum { | ||
| settings["insert_quorum"] = "auto" |
Contributor
There was a problem hiding this comment.
"auto" was introduced in ClickHouse 22.9, so on older version I suspect it may cause connect to fail. Since this is behind a feature flag, should be fine to add a note at the end of the Description like "Supported by ClickHouse 22.9+"
Contributor
|
two nits otherwise lgmt 👍 |
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:25 — with
GitHub Actions
Waiting
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:25 — with
GitHub Actions
Waiting
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:26 — with
GitHub Actions
Waiting
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:26 — with
GitHub Actions
Waiting
Contributor
Author
@jgao54 thanks for review, addressed. |
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:27 — with
GitHub Actions
Waiting
andreyzhelnin-st
requested a deployment
to
external-contributor
August 11, 2026 09:27 — with
GitHub Actions
Waiting
jgao54
approved these changes
Aug 11, 2026
jgao54
enabled auto-merge (squash)
August 11, 2026 22:45
Contributor
Author
|
@jgao54 how can I proceed with merge ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a multi-replica self hosted ReplicatedMergeTree cluster, normalize populates the raw table and then reads it back via
INSERT ... SELECT. Those two statements can run on different connections/replicas (e.g. behind a load-balanced host), so the read may hit a replica that has not yet replicated the just-written raw parts.select_sequential_consistency(already set on reads) only guarantees visibility of quorum-inserted blocks. With the defaultinsert_quorum=0, it enforces nothing, so the normalizeSELECTcan return fewer/zero rows, mark the batch processed, advance the normalize pointer, and silently drop those rows permanently.Fix
insert_quorum=autoandinsert_quorum_parallel=0(required for sequential-consistency reads to be honored) onReplicatedclusters. Gated behind a new dynamic settingPEERDB_CLICKHOUSE_ENABLE_REPLICATED_QUORUM(defaulttrue).select_sequential_consistency=1alongside the quorum writes so the read side of the read-your-writes contract stays paired with them regardless of the global settings default.Test plan