Skip to content

Commit 108b036

Browse files
zaidshabbir25mason-sharp
authored andcommitted
ACE-204 Improve mtree build message on an empty table
1 parent 9e658ad commit 108b036

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

internal/consistency/mtree/merkle.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,7 @@ func (m *MerkleTreeTask) BuildMtree() (err error) {
15021502
logger.Info("Getting row estimates from all nodes...")
15031503
var maxRows int64
15041504
var refNode map[string]any
1505+
successfulEstimates := 0
15051506
for _, nodeInfo := range m.ClusterNodes {
15061507
pool, err := auth.GetSizedClusterNodeConnection(nodeInfo, auth.ConnectionOptions{PoolSize: poolSize})
15071508
if err != nil {
@@ -1517,16 +1518,30 @@ func (m *MerkleTreeTask) BuildMtree() (err error) {
15171518
logger.Warn("Warning: Could not get row estimate from node %s: %v", nodeInfo["Name"], err)
15181519
continue
15191520
}
1521+
successfulEstimates++
15201522

1521-
if count > maxRows {
1523+
// Seed refNode on the first successful estimate so an all-empty table
1524+
// (every count == 0) still resolves a reference node instead of being
1525+
// mistaken for a total estimate failure below.
1526+
if refNode == nil || count > maxRows {
15221527
maxRows = count
15231528
refNode = nodeInfo
15241529
}
15251530
}
15261531

1527-
if refNode == nil {
1532+
if successfulEstimates == 0 {
1533+
for _, p := range pools {
1534+
p.Close()
1535+
}
15281536
return fmt.Errorf("could not determine a reference node; failed to get row estimates from all nodes")
15291537
}
1538+
1539+
if maxRows == 0 {
1540+
for _, p := range pools {
1541+
p.Close()
1542+
}
1543+
return fmt.Errorf("table %s has 0 rows on all nodes; add data before building a Merkle tree", m.QualifiedTableName)
1544+
}
15301545
logger.Info("Using node %s as the reference for defining block ranges.", refNode["Name"])
15311546

15321547
if len(blockRanges) == 0 {
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// ///////////////////////////////////////////////////////////////////////////
2+
//
3+
// # ACE - Active Consistency Engine
4+
//
5+
// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/)
6+
//
7+
// This software is released under the PostgreSQL License:
8+
// https://opensource.org/license/postgresql
9+
//
10+
// ///////////////////////////////////////////////////////////////////////////
11+
12+
package integration
13+
14+
import (
15+
"context"
16+
"fmt"
17+
"testing"
18+
19+
"github.com/jackc/pgx/v5"
20+
"github.com/jackc/pgx/v5/pgxpool"
21+
"github.com/stretchr/testify/require"
22+
)
23+
24+
// mtree build on a table that is empty on every node must fail with a clear,
25+
// actionable "0 rows on all nodes" message -- not the internal "could not
26+
// determine a reference node" error that leaves the user unsure whether the
27+
// table is missing, the nodes are unreachable, or something is broken.
28+
func TestBuildMtreeFailsClearlyOnEmptyTable(t *testing.T) {
29+
ctx := context.Background()
30+
tableName := "mtree_empty_table_test"
31+
qualifiedTable := fmt.Sprintf("%s.%s", testSchema, tableName)
32+
safeTable := pgx.Identifier{testSchema, tableName}.Sanitize()
33+
nodes := []string{serviceN1, serviceN2}
34+
35+
// Create the table on both nodes but insert no rows.
36+
for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} {
37+
_, err := pool.Exec(ctx, "CREATE TABLE IF NOT EXISTS "+safeTable+" (id INT PRIMARY KEY, payload TEXT)") // nosemgrep
38+
require.NoError(t, err)
39+
}
40+
t.Cleanup(func() {
41+
for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} {
42+
_, _ = pool.Exec(ctx, "DROP TABLE IF EXISTS "+safeTable) // nosemgrep
43+
}
44+
})
45+
46+
task := newTestMerkleTreeTask(t, qualifiedTable, nodes)
47+
require.NoError(t, task.RunChecks(false))
48+
require.NoError(t, task.MtreeInit())
49+
t.Cleanup(func() { _ = task.MtreeTeardown() })
50+
51+
err := task.BuildMtree()
52+
require.Error(t, err, "build on an empty table must fail")
53+
require.Contains(t, err.Error(), "has 0 rows on all nodes",
54+
"error must name the real cause: the table is empty")
55+
require.Contains(t, err.Error(), qualifiedTable,
56+
"error must name the offending table")
57+
require.NotContains(t, err.Error(), "could not determine a reference node",
58+
"the internal reference-node error must no longer leak for empty tables")
59+
}

0 commit comments

Comments
 (0)