Skip to content

Commit 2f02e5f

Browse files
mason-sharpclaude
andcommitted
Fix silent table exclusion in schema-diff and repset-diff
Tables that failed during per-table diff (e.g. missing primary key) were silently dropped from the report — only a logger.Warn on stderr, with overall status still COMPLETED. This made it possible for a user to see "no differences" while an entire table was never compared. Changes: - Add end-of-run summary listing identical, skipped, differed, missing, and errored tables with error reasons (shared via DiffSummary) - Set task status to FAILED when any table errors - Query tables from all nodes (not just the first) and report tables not present on every node - Support uni-directional repset-diff where the repset only exists on the publisher node - Add integration tests covering all summary categories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d51f785 commit 2f02e5f

5 files changed

Lines changed: 753 additions & 65 deletions

File tree

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
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 diff
13+
14+
import (
15+
"fmt"
16+
"strings"
17+
18+
"github.com/pgedge/ace/pkg/logger"
19+
)
20+
21+
// MissingTableInfo records a table that was not found on every node.
22+
type MissingTableInfo struct {
23+
Table string // schema-qualified table name
24+
PresentOn []string // node names where the table exists
25+
MissingFrom []string // node names where the table does not exist
26+
}
27+
28+
// FailedTableInfo records a table whose diff failed along with the reason.
29+
type FailedTableInfo struct {
30+
Table string // schema-qualified table name
31+
Err error // the error that caused the failure
32+
}
33+
34+
// DiffSummary collects per-table outcomes during a schema-diff or repset-diff
35+
// run and prints a human-readable summary at the end.
36+
type DiffSummary struct {
37+
MatchedTables []string
38+
DifferedTables []string
39+
FailedTables []FailedTableInfo
40+
MissingTables []MissingTableInfo
41+
SkippedTables []string
42+
}
43+
44+
// PrintAndFinalize logs the summary and returns an error if any tables failed.
45+
// The label identifies the scope (e.g. "schema public" or "repset default").
46+
func (s *DiffSummary) PrintAndFinalize(commandName, label string) error {
47+
total := len(s.MatchedTables) + len(s.DifferedTables) + len(s.FailedTables) +
48+
len(s.SkippedTables) + len(s.MissingTables)
49+
logger.Info("%s summary: %d table(s) in %s", commandName, total, label)
50+
if len(s.MatchedTables) > 0 {
51+
logger.Info(" %d table(s) are identical:", len(s.MatchedTables))
52+
for _, t := range s.MatchedTables {
53+
logger.Info(" - %s", t)
54+
}
55+
}
56+
if len(s.SkippedTables) > 0 {
57+
logger.Info(" %d table(s) were skipped:", len(s.SkippedTables))
58+
for _, t := range s.SkippedTables {
59+
logger.Info(" - %s", t)
60+
}
61+
}
62+
if len(s.DifferedTables) > 0 {
63+
logger.Warn(" %d table(s) have differences:", len(s.DifferedTables))
64+
for _, t := range s.DifferedTables {
65+
logger.Warn(" - %s", t)
66+
}
67+
}
68+
if len(s.MissingTables) > 0 {
69+
logger.Warn(" %d table(s) not present on all nodes:", len(s.MissingTables))
70+
for _, mt := range s.MissingTables {
71+
logger.Warn(" - %s found on [%s] but missing from [%s]",
72+
mt.Table,
73+
strings.Join(mt.PresentOn, ", "),
74+
strings.Join(mt.MissingFrom, ", "))
75+
}
76+
}
77+
if len(s.FailedTables) > 0 {
78+
logger.Error(" %d table(s) encountered errors and could not be compared:", len(s.FailedTables))
79+
var names []string
80+
for _, ft := range s.FailedTables {
81+
logger.Error(" - %s: %v", ft.Table, ft.Err)
82+
names = append(names, ft.Table)
83+
}
84+
return fmt.Errorf("%d table(s) failed during %s: %v", len(s.FailedTables), commandName, names)
85+
}
86+
87+
return nil
88+
}

internal/consistency/diff/repset_diff.go

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"fmt"
1818
"maps"
1919
"os"
20+
"sort"
2021
"strconv"
2122
"strings"
2223
"time"
@@ -43,6 +44,7 @@ type RepsetDiffCmd struct {
4344
SkipFile string
4445
skipTablesList []string
4546
tableList []string
47+
missingTables []MissingTableInfo
4648
nodeList []string
4749
clusterNodes []map[string]any
4850
database types.Database
@@ -129,38 +131,64 @@ func (c *RepsetDiffCmd) RunChecks(skipValidation bool) error {
129131
return fmt.Errorf("no nodes found in cluster config")
130132
}
131133

132-
firstNode := c.clusterNodes[0]
134+
// Query repset tables from every node that has the repset and build a union.
135+
// In a uni-directional setup the repset may only exist on the publisher, but
136+
// the tables themselves exist on all nodes, so we still diff them across
137+
// every node.
138+
tableSet := make(map[string]bool)
139+
var nodesWithRepset int
140+
141+
for _, nodeInfo := range c.clusterNodes {
142+
nodeName := nodeInfo["Name"].(string)
143+
144+
nodeWithDBInfo := make(map[string]any)
145+
maps.Copy(nodeWithDBInfo, nodeInfo)
146+
utils.ApplyDatabaseCredentials(nodeWithDBInfo, c.database)
147+
if portVal, ok := nodeWithDBInfo["Port"]; ok {
148+
if portFloat, isFloat := portVal.(float64); isFloat {
149+
nodeWithDBInfo["Port"] = strconv.Itoa(int(portFloat))
150+
}
151+
}
133152

134-
nodeWithDBInfo := make(map[string]any)
135-
maps.Copy(nodeWithDBInfo, firstNode)
136-
utils.ApplyDatabaseCredentials(nodeWithDBInfo, c.database)
153+
pool, err := auth.GetClusterNodeConnection(c.Ctx, nodeWithDBInfo, c.connOpts())
154+
if err != nil {
155+
return fmt.Errorf("could not connect to node %s: %w", nodeName, err)
156+
}
137157

138-
if portVal, ok := nodeWithDBInfo["Port"]; ok {
139-
if portFloat, isFloat := portVal.(float64); isFloat {
140-
nodeWithDBInfo["Port"] = strconv.Itoa(int(portFloat))
158+
repsetExists, err := queries.CheckRepSetExists(c.Ctx, pool, c.RepsetName)
159+
if err != nil {
160+
pool.Close()
161+
return fmt.Errorf("could not check if repset exists on node %s: %w", nodeName, err)
141162
}
142-
}
163+
if !repsetExists {
164+
pool.Close()
165+
logger.Warn("repset %s not found on node %s, skipping for table discovery", c.RepsetName, nodeName)
166+
continue
167+
}
168+
nodesWithRepset++
143169

144-
pool, err := auth.GetClusterNodeConnection(c.Ctx, nodeWithDBInfo, c.connOpts())
145-
if err != nil {
146-
return fmt.Errorf("could not connect to database: %w", err)
147-
}
148-
defer pool.Close()
170+
tables, err := queries.GetTablesInRepSet(c.Ctx, pool, c.RepsetName)
171+
pool.Close()
172+
if err != nil {
173+
return fmt.Errorf("could not get tables in repset on node %s: %w", nodeName, err)
174+
}
149175

150-
repsetExists, err := queries.CheckRepSetExists(c.Ctx, pool, c.RepsetName)
151-
if err != nil {
152-
return fmt.Errorf("could not check if repset exists: %w", err)
176+
for _, t := range tables {
177+
tableSet[t] = true
178+
}
153179
}
154-
if !repsetExists {
155-
return fmt.Errorf("repset %s not found", c.RepsetName)
180+
181+
if nodesWithRepset == 0 {
182+
return fmt.Errorf("repset %s not found on any node", c.RepsetName)
156183
}
157184

158-
tables, err := queries.GetTablesInRepSet(c.Ctx, pool, c.RepsetName)
159-
if err != nil {
160-
return fmt.Errorf("could not get tables in repset: %w", err)
185+
var allTables []string
186+
for t := range tableSet {
187+
allTables = append(allTables, t)
161188
}
189+
sort.Strings(allTables)
162190

163-
c.tableList = tables
191+
c.tableList = allTables
164192

165193
if len(c.tableList) == 0 {
166194
return fmt.Errorf("no tables found in repset %s", c.RepsetName)
@@ -230,8 +258,10 @@ func RepsetDiff(task *RepsetDiffCmd) (err error) {
230258
}
231259
}
232260

233-
var tablesProcessed, tablesFailed, tablesSkipped int
234-
var failedTables []string
261+
var tablesProcessed, tablesFailed int
262+
var failedTables []FailedTableInfo
263+
var skippedTables []string
264+
var summary DiffSummary
235265

236266
defer func() {
237267
finishedAt := time.Now()
@@ -249,10 +279,14 @@ func RepsetDiff(task *RepsetDiffCmd) (err error) {
249279
"tables_total": len(task.tableList),
250280
"tables_diffed": tablesProcessed,
251281
"tables_failed": tablesFailed,
252-
"tables_skipped": tablesSkipped,
282+
"tables_skipped": len(skippedTables),
253283
}
254284
if len(failedTables) > 0 {
255-
ctx["failed_tables"] = failedTables
285+
names := make([]string, len(failedTables))
286+
for i, ft := range failedTables {
287+
names[i] = ft.Table
288+
}
289+
ctx["failed_tables"] = names
256290
}
257291
if err != nil {
258292
ctx["error"] = err.Error()
@@ -293,7 +327,7 @@ func RepsetDiff(task *RepsetDiffCmd) (err error) {
293327
}
294328
}
295329
if skipped {
296-
tablesSkipped++
330+
skippedTables = append(skippedTables, tableName)
297331
continue
298332
}
299333

@@ -320,27 +354,35 @@ func RepsetDiff(task *RepsetDiffCmd) (err error) {
320354
if err := tdTask.Validate(); err != nil {
321355
logger.Warn("validation for table %s failed: %v", tableName, err)
322356
tablesFailed++
323-
failedTables = append(failedTables, tableName)
357+
failedTables = append(failedTables, FailedTableInfo{Table: tableName, Err: err})
324358
continue
325359
}
326360

327361
if err := tdTask.RunChecks(true); err != nil {
328362
logger.Warn("checks for table %s failed: %v", tableName, err)
329363
tablesFailed++
330-
failedTables = append(failedTables, tableName)
364+
failedTables = append(failedTables, FailedTableInfo{Table: tableName, Err: err})
331365
continue
332366
}
333367
if err := tdTask.ExecuteTask(); err != nil {
334368
logger.Warn("error during comparison for table %s: %v", tableName, err)
335369
tablesFailed++
336-
failedTables = append(failedTables, tableName)
370+
failedTables = append(failedTables, FailedTableInfo{Table: tableName, Err: err})
337371
continue
338372
}
339373

374+
if len(tdTask.DiffResult.NodeDiffs) > 0 {
375+
summary.DifferedTables = append(summary.DifferedTables, tableName)
376+
} else {
377+
summary.MatchedTables = append(summary.MatchedTables, tableName)
378+
}
340379
tablesProcessed++
341380
}
342381

343-
return nil
382+
summary.FailedTables = failedTables
383+
summary.SkippedTables = skippedTables
384+
summary.MissingTables = task.missingTables
385+
return summary.PrintAndFinalize("Repset diff", "repset "+task.RepsetName)
344386
}
345387

346388
func (task *RepsetDiffCmd) CloneForSchedule(ctx context.Context) *RepsetDiffCmd {

0 commit comments

Comments
 (0)