From 6cc5374bdf87b52412695cbd81514c67931d25ae Mon Sep 17 00:00:00 2001 From: Zaid-edge Date: Thu, 16 Jul 2026 20:33:32 +0500 Subject: [PATCH 1/2] ACE-204 Improve mtree build message on an empty table --- internal/consistency/mtree/merkle.go | 19 ++++++- tests/integration/mtree_empty_table_test.go | 59 +++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 tests/integration/mtree_empty_table_test.go diff --git a/internal/consistency/mtree/merkle.go b/internal/consistency/mtree/merkle.go index a2ab646..3add56e 100644 --- a/internal/consistency/mtree/merkle.go +++ b/internal/consistency/mtree/merkle.go @@ -1502,6 +1502,7 @@ func (m *MerkleTreeTask) BuildMtree() (err error) { logger.Info("Getting row estimates from all nodes...") var maxRows int64 var refNode map[string]any + successfulEstimates := 0 for _, nodeInfo := range m.ClusterNodes { pool, err := auth.GetSizedClusterNodeConnection(nodeInfo, auth.ConnectionOptions{PoolSize: poolSize}) if err != nil { @@ -1517,16 +1518,30 @@ func (m *MerkleTreeTask) BuildMtree() (err error) { logger.Warn("Warning: Could not get row estimate from node %s: %v", nodeInfo["Name"], err) continue } + successfulEstimates++ - if count > maxRows { + // Seed refNode on the first successful estimate so an all-empty table + // (every count == 0) still resolves a reference node instead of being + // mistaken for a total estimate failure below. + if refNode == nil || count > maxRows { maxRows = count refNode = nodeInfo } } - if refNode == nil { + if successfulEstimates == 0 { + for _, p := range pools { + p.Close() + } return fmt.Errorf("could not determine a reference node; failed to get row estimates from all nodes") } + + if maxRows == 0 { + for _, p := range pools { + p.Close() + } + return fmt.Errorf("table %s has 0 rows on all nodes; add data before building a Merkle tree", m.QualifiedTableName) + } logger.Info("Using node %s as the reference for defining block ranges.", refNode["Name"]) if len(blockRanges) == 0 { diff --git a/tests/integration/mtree_empty_table_test.go b/tests/integration/mtree_empty_table_test.go new file mode 100644 index 0000000..aaae676 --- /dev/null +++ b/tests/integration/mtree_empty_table_test.go @@ -0,0 +1,59 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package integration + +import ( + "context" + "fmt" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/require" +) + +// mtree build on a table that is empty on every node must fail with a clear, +// actionable "0 rows on all nodes" message -- not the internal "could not +// determine a reference node" error that leaves the user unsure whether the +// table is missing, the nodes are unreachable, or something is broken. +func TestBuildMtreeFailsClearlyOnEmptyTable(t *testing.T) { + ctx := context.Background() + tableName := "mtree_empty_table_test" + qualifiedTable := fmt.Sprintf("%s.%s", testSchema, tableName) + safeTable := pgx.Identifier{testSchema, tableName}.Sanitize() + nodes := []string{serviceN1, serviceN2} + + // Create the table on both nodes but insert no rows. + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + _, err := pool.Exec(ctx, "CREATE TABLE IF NOT EXISTS "+safeTable+" (id INT PRIMARY KEY, payload TEXT)") // nosemgrep + require.NoError(t, err) + } + t.Cleanup(func() { + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + _, _ = pool.Exec(ctx, "DROP TABLE IF EXISTS "+safeTable) // nosemgrep + } + }) + + task := newTestMerkleTreeTask(t, qualifiedTable, nodes) + require.NoError(t, task.RunChecks(false)) + require.NoError(t, task.MtreeInit()) + t.Cleanup(func() { _ = task.MtreeTeardown() }) + + err := task.BuildMtree() + require.Error(t, err, "build on an empty table must fail") + require.Contains(t, err.Error(), "has 0 rows on all nodes", + "error must name the real cause: the table is empty") + require.Contains(t, err.Error(), qualifiedTable, + "error must name the offending table") + require.NotContains(t, err.Error(), "could not determine a reference node", + "the internal reference-node error must no longer leak for empty tables") +} From 8bce1365e37f72a5ded42b85a027bf3e08481463 Mon Sep 17 00:00:00 2001 From: Zaid-edge Date: Fri, 17 Jul 2026 14:15:53 +0500 Subject: [PATCH 2/2] fix(mtree): detect UPDATE-UPDATE conflicts missed by overlapping leaves (ACE-205) mtree table-diff could report divergent nodes as identical: the same primary key holding different non-key data on two nodes produced matching Merkle-tree root hashes, so the diff printed "Merkle trees are identical" while table-diff correctly found the difference. This is the core UPDATE-UPDATE conflict that active-active clusters produce, silently reported as consistent. Root cause: leaf hashing used a closed ("<=") upper bound on block ranges, but a block's range_end is the EXCLUSIVE start of the next block -- the build offsets query pairs bounds with LEAD (each range_end is the following block's range_start) and splitBlocks sets range_end to a split point that GetBulkSplitPoints returns as the first row of the next chunk. The closed bound double-counted every boundary row into two adjacent leaves. When a table's rows collapsed into overlapping leaves that hashed identically (most visibly a single-row table, where the row lands in both [pk,pk] and [pk,inf)), the XOR-based parent hash cancelled the duplicate siblings to zero on every node, so divergent data yielded matching roots. Leaf hashing now uses an exclusive ("<") upper bound, matching the boundaries the build and split paths already produce, so each row belongs to exactly one leaf. Merkle trees built before this fix must be rebuilt (mtree build) to pick up the corrected layout. Adds an integration regression test (single-row and multi-row-boundary cases) asserting mtree diff agrees with table-diff. Verified against the full mtree integration suite plus db/queries and consistency unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- db/queries/queries.go | 15 +- docs/CHANGELOG.md | 15 ++ .../integration/mtree_update_conflict_test.go | 128 ++++++++++++++++++ 3 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 tests/integration/mtree_update_conflict_test.go diff --git a/db/queries/queries.go b/db/queries/queries.go index 357410c..8d50743 100644 --- a/db/queries/queries.go +++ b/db/queries/queries.go @@ -671,10 +671,17 @@ func BlockHashSQL(schema, table string, primaryKeyCols []string, mode string, in endPlaceholders[i] = fmt.Sprintf("$%d", paramIndex) paramIndex++ } - operator := "<=" - if mode == "TD_BLOCK_HASH" { - operator = "<" - } + // The upper bound is always EXCLUSIVE. Block boundaries are the start of + // the *next* block: the build offsets query pairs bounds with LEAD (each + // range_end is the following block's range_start), and splitBlocks sets a + // block's range_end to the split point, which GetBulkSplitPoints returns + // as the first row of the next chunk. A closed "<=" upper bound therefore + // double-counts every boundary row into two adjacent leaves. When a whole + // table collapses into overlapping leaves that hash identically (e.g. a + // single row landing in both [pk,pk] and [pk,∞)), the XOR-based parent + // hash cancels the duplicate siblings to zero on every node, so divergent + // data produces matching root hashes and the diff misses real conflicts. + operator := "<" var upperExpr string if len(primaryKeyCols) == 1 { upperExpr = fmt.Sprintf("%s %s %s", pkComparisonExpression, operator, endPlaceholders[0]) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a68d209..3588cb5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -54,6 +54,21 @@ This release focuses primarily on Merkle tree functionality. means "re-run or raise", since drain progress is durable). ### Fixed +- **`mtree table-diff` could report divergent nodes as identical (silent + false-negative).** Merkle-tree leaf hashing used a closed (`<=`) upper bound + on block ranges, but a block's `range_end` is the *exclusive* start of the + next block, so every boundary row was hashed into two adjacent leaves. When a + table's rows collapsed into overlapping leaves that hashed identically (most + visibly a single-row table, where the row lands in both `[pk, pk]` and + `[pk, ∞)`), the XOR-based parent hash cancelled the duplicate siblings to zero + on every node — so the same primary key holding *different* non-key data + produced matching root hashes and the diff reported "Merkle trees are + identical" while `table-diff` correctly found the difference. This is the + core UPDATE-UPDATE conflict active-active clusters produce. Leaf hashing now + uses an exclusive (`<`) upper bound, matching the boundaries the build and + split paths already produce, so each row belongs to exactly one leaf. + **Merkle trees built before this fix must be rebuilt (`mtree build`) to pick + up the corrected layout.** - **A bounded CDC drain could silently under-report divergence.** The drain treated a 1-second idle timeout as "caught up", so a node with a large backlog could leave its Merkle tree stale and the diff would report the diff --git a/tests/integration/mtree_update_conflict_test.go b/tests/integration/mtree_update_conflict_test.go new file mode 100644 index 0000000..3ea16d2 --- /dev/null +++ b/tests/integration/mtree_update_conflict_test.go @@ -0,0 +1,128 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package integration + +import ( + "context" + "fmt" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/require" +) + +// Regression: same PK exists on both nodes with different non-key data at BUILD +// time (an UPDATE-UPDATE conflict). mtree diff must report it, matching plain +// table-diff. +// +// The divergence exists before the tree is built -- unlike +// testMerkleTreeDiffModifiedRows, which builds on identical data and mutates +// afterward, so it never exercised the degenerate build-time range layout. +// +// The bug: a block's range_end is the EXCLUSIVE start of the next block, but +// leaf hashing used a closed "<=" upper bound, double-counting boundary rows +// into two adjacent leaves. A single-row table collapses into two identical +// overlapping leaves ([pk,pk] and [pk,inf)); the XOR-based parent hash cancels +// the duplicate siblings to zero on every node, so divergent data yielded +// matching root hashes and the diff reported "trees identical". The single-row +// case below reproduced it; the multi-row case guards the boundary. +func TestMtreeDiff_UpdateUpdateConflictAtBuildTime(t *testing.T) { + t.Run("SingleRow", func(t *testing.T) { + runMtreeUpdateConflictCase(t, "mtree_uu_conflict_1", []int{1}) + }) + t.Run("MultiRowBoundary", func(t *testing.T) { + // The divergent row (id=3) is the middle block boundary under BlockSize=2. + runMtreeUpdateConflictCase(t, "mtree_uu_conflict_n", []int{1, 2, 3, 4, 5}) + }) +} + +// runMtreeUpdateConflictCase seeds ids on both nodes, diverging the LAST id's +// non-key column, then asserts both table-diff and mtree diff report exactly +// one divergent row. +func runMtreeUpdateConflictCase(t *testing.T, tableName string, ids []int) { + ctx := context.Background() + qualifiedTable := fmt.Sprintf("%s.%s", testSchema, tableName) + nodes := []string{serviceN1, serviceN2} + divergentID := ids[len(ids)-1] + + for i, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + nodeName := pgCluster.ClusterNodes[i]["Name"].(string) + _, err := pool.Exec(ctx, fmt.Sprintf( // nosemgrep + "CREATE TABLE IF NOT EXISTS %s (id INT PRIMARY KEY, name VARCHAR)", qualifiedTable)) + require.NoError(t, err, "create table on %s", nodeName) + _, err = pool.Exec(ctx, fmt.Sprintf( // nosemgrep + "SELECT spock.repset_add_table('default', '%s')", qualifiedTable)) + require.NoError(t, err, "add to repset on %s", nodeName) + } + t.Cleanup(func() { + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + pool.Exec(ctx, fmt.Sprintf("DROP TABLE IF EXISTS %s CASCADE", qualifiedTable)) // nosemgrep + } + }) + + // Seed identical rows on both nodes, then diverge divergentID's non-key + // column. repair_mode(true) keeps these writes from replicating. + seed := func(pool *pgxpool.Pool, divergentName string) { + tx, err := pool.Begin(ctx) + require.NoError(t, err) + _, err = tx.Exec(ctx, "SELECT spock.repair_mode(true)") + require.NoError(t, err) + for _, id := range ids { + name := fmt.Sprintf("name-%d", id) + if id == divergentID { + name = divergentName + } + _, err = tx.Exec(ctx, fmt.Sprintf( // nosemgrep + "INSERT INTO %s (id, name) VALUES ($1, $2)", qualifiedTable), id, name) + require.NoError(t, err) + } + _, err = tx.Exec(ctx, "SELECT spock.repair_mode(false)") + require.NoError(t, err) + require.NoError(t, tx.Commit(ctx)) + } + seed(pgCluster.Node1Pool, "zaid") + seed(pgCluster.Node2Pool, "shabbir") + + // Baseline: plain table-diff must find the divergence. + tdTask := newTestTableDiffTask(t, qualifiedTable, nodes) + require.NoError(t, tdTask.RunChecks(false)) + require.NoError(t, tdTask.ExecuteTask()) + require.Equal(t, 1, sumDiffRows(tdTask.DiffResult.Summary.DiffRowsCount), + "table-diff must find the id=%d conflict", divergentID) + + // mtree diff on the same build-time divergence must ALSO find it. + mtreeTask := newTestMerkleTreeTask(t, qualifiedTable, nodes) + mtreeTask.BlockSize = 2 + mtreeTask.OverrideBlockSize = true + require.NoError(t, mtreeTask.RunChecks(false)) + require.NoError(t, mtreeTask.MtreeInit()) + t.Cleanup(func() { _ = mtreeTask.MtreeTeardown() }) + require.NoError(t, mtreeTask.BuildMtree()) + + diffTask := newTestMerkleTreeTask(t, qualifiedTable, nodes) + diffTask.Mode = "diff" + diffTask.Output = "json" + diffTask.BlockSize = 2 + diffTask.OverrideBlockSize = true + require.NoError(t, diffTask.RunChecks(false)) + require.NoError(t, diffTask.DiffMtree()) + require.Equal(t, 1, sumDiffRows(diffTask.DiffResult.Summary.DiffRowsCount), + "mtree diff must ALSO find the id=%d conflict", divergentID) +} + +func sumDiffRows(counts map[string]int) int { + total := 0 + for _, c := range counts { + total += c + } + return total +}