Skip to content

fix(clickhouse): quorum writes on replicated clusters to stop normalize dropping rows - #4676

Open
andreyzhelnin-st wants to merge 5 commits into
PeerDB-io:mainfrom
andreyzhelnin-st:st-fixes-v0.37.3
Open

fix(clickhouse): quorum writes on replicated clusters to stop normalize dropping rows#4676
andreyzhelnin-st wants to merge 5 commits into
PeerDB-io:mainfrom
andreyzhelnin-st:st-fixes-v0.37.3

Conversation

@andreyzhelnin-st

@andreyzhelnin-st andreyzhelnin-st commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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 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.

Fix

  • Write raw/normalize inserts with insert_quorum=auto and insert_quorum_parallel=0 (required for sequential-consistency reads to be honored) on Replicated clusters. Gated behind a new dynamic setting PEERDB_CLICKHOUSE_ENABLE_REPLICATED_QUORUM (default true).
  • Explicitly pin select_sequential_consistency=1 alongside 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

  • Verified against a multi-replica ReplicatedMergeTree cluster that normalize no longer drops rows under replica lag
  • Existing ClickHouse connector tests pass

andreyzhelnin-st and others added 2 commits August 2, 2026 14:38
…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
andreyzhelnin-st requested a review from a team as a code owner August 7, 2026 07:22
@andreyzhelnin-st

Copy link
Copy Markdown
Contributor Author

Could you please review this @jgao54 - it works good in our prod for a few days?

@andreyzhelnin-st

Copy link
Copy Markdown
Contributor Author

@serprex could you please have a look?

Comment thread flow/internal/dynamicconf.go Outdated
"(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",

@jgao54 jgao54 Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

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"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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+"

@jgao54

jgao54 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

two nits otherwise lgmt 👍

Comment thread flow/internal/dynamicconf.go Outdated
@andreyzhelnin-st

Copy link
Copy Markdown
Contributor Author

two nits otherwise lgmt 👍

@jgao54 thanks for review, addressed.

@jgao54
jgao54 enabled auto-merge (squash) August 11, 2026 22:45
@andreyzhelnin-st

Copy link
Copy Markdown
Contributor Author

@jgao54 how can I proceed with merge ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants