diff --git a/ace.sample.yaml b/ace.sample.yaml index f80dbe35..9cbc85f2 100644 --- a/ace.sample.yaml +++ b/ace.sample.yaml @@ -22,6 +22,7 @@ postgres: table_diff: concurrency_factor: 0.5 max_diff_rows: 1000000 + max_html_rows: 10000 # max rows per node pair in the HTML report min_diff_block_size: 1 max_diff_block_size: 1000000 diff_block_size: 1000 @@ -52,6 +53,8 @@ mtree: max_block_size: 1000000 # Max differing rows collected per node pair; 0 or absent means unbounded. max_diff_rows: 1000000 + # Max rows per node pair in the HTML report (--output html). + max_html_rows: 10000 # Example scheduler configuration: # schedule_jobs: diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6be6843b..7967e020 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,30 @@ All notable changes to ACE will be captured in this document. This project follo ## [Unreleased] ### Fixed +- **`table-diff --output html` was killed by the OOM killer on large diffs.** + The HTML writer built the whole report in memory: a copy of the diff as + JSON, the markup of every row, and the final document in one buffer. With + several kilobytes of markup per row, a diff of 493,200 rows used about 4 GB + before the process was killed. + The writer now streams the report to disk one row at a time. It also shows + at most `max_html_rows` rows for each node pair (default `10000`, set in + `table_diff` and `mtree.diff` in `ace.yaml`, or with `--max-html-rows`), + because a report with hundreds of thousands of rows is too large for a + browser anyway. A truncated report says so at the top and in each node pair + section. A repair plan built in it has rules only for the rows shown and + `default_action: skip`, so `table-repair` does not change the rows that the + report did not show; the YAML starts with a comment that says this. With + three or more nodes this holds per key only: a plan rule is not tied to a + node pair, so it also acts on a key that the report hides in another pair. + The JSON report is not changed and always contains every row. +- **HTML report: repair plans named the wrong rows for some primary keys.** + The page script read keys as JavaScript numbers, so a bigint above 2^53 + could name the neighbouring row, and a text key such as `"007"` became the + number 7 and matched nothing. The plan now uses the keys exactly as the + diff file has them. +- **HTML report: rows with primary keys that mix numbers and text were sorted + in a different order on each run.** The key comparison now puts numbers + before text, so the order is stable. - **Spock `add_node` failed on clusters where `mtree init` had been run.** `mtree init` created a `#` operator on `bytea_xor` without a schema name, so it landed in `public` while the function stayed in the ACE schema. Spock's diff --git a/docs/commands/diff/table-diff.md b/docs/commands/diff/table-diff.md index 4bf941ce..d30d1542 100644 --- a/docs/commands/diff/table-diff.md +++ b/docs/commands/diff/table-diff.md @@ -20,6 +20,7 @@ This command compares the data in the specified table across nodes in a cluster | `--concurrency-factor ` | `-c` | CPU ratio for concurrency (0.0–4.0, e.g. 0.5 uses half of available CPUs). Default `0.5`. | | `--compare-unit-size ` | `-u` | Recursive split size for mismatched blocks. Default `10000`. | | `--output ` | `-o` | Report format. Default `json`. When `html`, both JSON and HTML files share the same timestamped prefix. | +| `--max-html-rows ` | | Maximum number of rows for each node pair in the HTML report. Default `0`, which means `table_diff.max_html_rows` from `ace.yaml`, or `10000` when that is not set either. There is no setting for "no limit"; set a large number instead. The JSON report always contains every row. | | `--nodes ` | `-n` | Comma-separated node list or `all`. Up to three-way diffs are supported. | | `--table-filter ` | `-F` | Optional SQL `WHERE` clause applied on every node before hashing. | | `--against-origin ` | | Limit the diff to rows whose `node_origin` matches this Spock node id or name (useful for failed-node recovery). | @@ -119,6 +120,25 @@ ace table-diff my-cluster public._events \ summary as both the raw filter and the effective filter (which also includes `--against-origin`/`--until` if set). 3. Prefer `--output html` when you'll manually review diffs. + - The HTML report shows at most `max_html_rows` rows for each node pair + (default `10000`). A row is one primary key: a value difference or a + row missing on one node. Each row takes several kilobytes of markup, so + a report with hundreds of thousands of rows is too large for a browser. + When the diff has more rows, the report says so at the top and in each + node pair section, and the full list stays in the JSON file. Rows are + taken in report order: value differences, then rows missing on the + second node, then rows missing on the first node. + - A repair plan that you build in a truncated report has rules only for + the rows shown, and its `default_action` is `skip`. So when you run + `table-repair` with that plan and the full diff file, the rows that the + report did not show are not changed. The downloaded YAML starts with a + comment that says this. To repair every row, run `table-diff` again with + a larger `--max-html-rows`, or write the plan by hand. + - Known limit: a plan rule matches a primary key and a kind of + difference, not a node pair. With three or more nodes, a rule for a + key shown in one pair also acts on the same key in another pair, even + where the report hides it there. Check such keys before you run + `table-repair`. 4. Use `--override-block-size` sparingly; the guardrails in `ace.yaml` prevent allocations that can overwhelm memory. 5. The `max_diff_rows` setting caps the number of differing rows that diff --git a/docs/commands/mtree/mtree-table-diff.md b/docs/commands/mtree/mtree-table-diff.md index 363cda06..60b57d33 100644 --- a/docs/commands/mtree/mtree-table-diff.md +++ b/docs/commands/mtree/mtree-table-diff.md @@ -31,6 +31,7 @@ holding the node's slot, then re-run, if you need a guaranteed-current drain. | `--nodes` | `-n` | Nodes to include (comma or `all`) | `all` | | `--max-cpu-ratio` | `-m` | Max CPU ratio | `0.5` | | `--output` | `-o` | `json` or `html` | `json` | +| `--max-html-rows` | | Max rows per node pair in the HTML report (`0` = use `mtree.diff.max_html_rows`, or `10000`) | `0` | | `--skip-cdc` | `-U` | Skip CDC processing (only rehash and compare) | `false` | | `--cdc-timeout` | | Seconds to drain CDC before giving up (`0` = use `cdc_processing_timeout` / default) | `0` | | `--quiet` | `-q` | Suppress output | `false` | @@ -45,6 +46,13 @@ holding the node's slot, then re-run, if you need a guaranteed-current drain. **Notes** - With `--output html`, both JSON and HTML reports are generated with matching timestamps. +- The HTML report shows at most `max_html_rows` rows for each node pair + (`mtree.diff.max_html_rows`, default `10000`). The JSON report always + contains every row. When the HTML report is truncated, it says so, and a + repair plan built in it has rules only for the rows shown and + `default_action: skip`, so `table-repair` does not change the other rows. + With three or more nodes this holds per key, not per node pair: see the + known limit in the `table-diff` docs. - The number of differing rows collected per node pair is bounded by `mtree.diff.max_diff_rows` (the shipped `ace.yaml` sets `1000000`; if the key is absent or `0`, the diff is unbounded). When the cap is reached, enumeration diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 400e45ce..4dbf49d9 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -142,6 +142,11 @@ func SetupCLI(version string) *cli.Command { tableDiffFlags := append(commonFlags, diffFlags...) tableDiffFlags = append(tableDiffFlags, + &cli.Int64Flag{ + Name: "max-html-rows", + Usage: "Max rows per node pair in the HTML report (0 = use max_html_rows from config, default 10000)", + Value: 0, + }, &cli.StringFlag{ Name: "table-filter", Aliases: []string{"F"}, @@ -376,6 +381,11 @@ func SetupCLI(version string) *cli.Command { mtreeUpdateFlags = append(mtreeUpdateFlags, commonFlags...) mtreeDiffFlags := []cli.Flag{ + &cli.Int64Flag{ + Name: "max-html-rows", + Usage: "Max rows per node pair in the HTML report (0 = use max_html_rows from config, default 10000)", + Value: 0, + }, &cli.Float64Flag{ Name: "max-cpu-ratio", Aliases: []string{"m"}, @@ -948,6 +958,7 @@ func TableDiffCLI(cmd *cli.Command) error { task.MaxConnections = cmd.Int("max-connections") task.CompareUnitSize = cmd.Int("compare-unit-size") task.Output = strings.ToLower(cmd.String("output")) + task.MaxHTMLRows = cmd.Int64("max-html-rows") task.Nodes = cmd.String("nodes") task.EnsurePgcrypto = cmd.Bool("ensure-pgcrypto") scheduleEnabled := cmd.Bool("schedule") @@ -1205,6 +1216,7 @@ func MtreeDiffCLI(cmd *cli.Command) error { task.QuietMode = cmd.Bool("quiet") task.MaxCpuRatio = cmd.Float64("max-cpu-ratio") task.Output = cmd.String("output") + task.MaxHTMLRows = cmd.Int64("max-html-rows") task.NoCDC = cmd.Bool("skip-cdc") task.CDCTimeoutSec = cmd.Int("cdc-timeout") task.Until = cmd.String("until") diff --git a/internal/cli/default_config.yaml b/internal/cli/default_config.yaml index f80dbe35..9cbc85f2 100644 --- a/internal/cli/default_config.yaml +++ b/internal/cli/default_config.yaml @@ -22,6 +22,7 @@ postgres: table_diff: concurrency_factor: 0.5 max_diff_rows: 1000000 + max_html_rows: 10000 # max rows per node pair in the HTML report min_diff_block_size: 1 max_diff_block_size: 1000000 diff_block_size: 1000 @@ -52,6 +53,8 @@ mtree: max_block_size: 1000000 # Max differing rows collected per node pair; 0 or absent means unbounded. max_diff_rows: 1000000 + # Max rows per node pair in the HTML report (--output html). + max_html_rows: 10000 # Example scheduler configuration: # schedule_jobs: diff --git a/internal/consistency/diff/table_diff.go b/internal/consistency/diff/table_diff.go index e35e2ea8..774c1c0a 100644 --- a/internal/consistency/diff/table_diff.go +++ b/internal/consistency/diff/table_diff.go @@ -105,6 +105,10 @@ type TableDiffTask struct { CompareUnitSize int MaxDiffRows int64 + // MaxHTMLRows limits how many entries the HTML report shows for each + // node pair. 0 means table_diff.max_html_rows from the config, and if + // that is not set either, utils.DefaultMaxHTMLRows. + MaxHTMLRows int64 DiffResult types.DiffOutput diffMutex sync.Mutex @@ -680,6 +684,16 @@ func (t *TableDiffTask) Validate() error { t.MaxDiffRows = cfg.TableDiff.MaxDiffRows } + if t.MaxHTMLRows < 0 { + return fmt.Errorf("max_html_rows must be >= 0") + } + if cfg.TableDiff.MaxHTMLRows < 0 { + return fmt.Errorf("table_diff.max_html_rows in the config must be >= 0, got %d", cfg.TableDiff.MaxHTMLRows) + } + if t.MaxHTMLRows == 0 && cfg.TableDiff.MaxHTMLRows > 0 { + t.MaxHTMLRows = cfg.TableDiff.MaxHTMLRows + } + if t.ConcurrencyFactor > 4.0 || t.ConcurrencyFactor <= 0 { return fmt.Errorf("invalid value range for concurrency_factor, must be > 0 and <= 4.0") } @@ -1053,6 +1067,7 @@ func (t *TableDiffTask) CloneForSchedule(ctx context.Context) *TableDiffTask { cloned.InvokeMethod = t.InvokeMethod cloned.CompareUnitSize = t.CompareUnitSize cloned.MaxDiffRows = t.MaxDiffRows + cloned.MaxHTMLRows = t.MaxHTMLRows cloned.EnsurePgcrypto = t.EnsurePgcrypto cloned.AgainstOrigin = t.AgainstOrigin cloned.Until = t.Until @@ -1641,7 +1656,7 @@ func (t *TableDiffTask) ExecuteTask() (err error) { t.AddPrimaryKeyToDiffSummary() - jsonPath, _, err := utils.WriteDiffReport(t.DiffResult, t.Schema, t.BaseTable, t.Output) + jsonPath, _, err := utils.WriteDiffReport(t.DiffResult, t.Schema, t.BaseTable, t.Output, t.MaxHTMLRows) if err != nil { return err } diff --git a/internal/consistency/diff/table_diff_html_rows_test.go b/internal/consistency/diff/table_diff_html_rows_test.go new file mode 100644 index 00000000..5c1f84d8 --- /dev/null +++ b/internal/consistency/diff/table_diff_html_rows_test.go @@ -0,0 +1,69 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # 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 diff + +import ( + "strings" + "testing" + + "github.com/pgedge/ace/pkg/config" +) + +// TestValidateMaxHTMLRows checks how Validate resolves max_html_rows: the +// task value wins, 0 falls back to table_diff.max_html_rows, and a negative +// value in either place is an error. Validate goes on to fail on other +// checks later (the task here is incomplete), so only the max_html_rows +// errors and the resolved value are checked. +func TestValidateMaxHTMLRows(t *testing.T) { + for _, tc := range []struct { + name string + task int64 + cfg int64 + want int64 + wantErrs string + }{ + {name: "task value wins", task: 7, cfg: 500, want: 7}, + {name: "config when task is 0", task: 0, cfg: 500, want: 500}, + {name: "both 0 leave the default to the writer", task: 0, cfg: 0, want: 0}, + {name: "negative task value", task: -1, cfg: 500, wantErrs: "max_html_rows must be >= 0"}, + {name: "negative config value", task: 0, cfg: -5, wantErrs: "table_diff.max_html_rows in the config must be >= 0"}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := &config.Config{} + cfg.TableDiff.MinBlockSize = 1 + cfg.TableDiff.MaxBlockSize = 1000000 + cfg.TableDiff.MaxHTMLRows = tc.cfg + config.Set(cfg) + t.Cleanup(func() { config.Set(nil) }) + + task := NewTableDiffTask() + task.ClusterName = "c" + task.QualifiedTableName = "public.t" + task.BlockSize = 1000 + task.MaxHTMLRows = tc.task + + err := task.Validate() + if tc.wantErrs != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErrs) { + t.Fatalf("Validate: got error %v, want one containing %q", err, tc.wantErrs) + } + return + } + if err != nil && strings.Contains(err.Error(), "max_html_rows") { + t.Fatalf("Validate: unexpected max_html_rows error: %v", err) + } + if task.MaxHTMLRows != tc.want { + t.Errorf("MaxHTMLRows after Validate: got %d, want %d", task.MaxHTMLRows, tc.want) + } + }) + } +} diff --git a/internal/consistency/mtree/merkle.go b/internal/consistency/mtree/merkle.go index eb2f5ede..054fc816 100644 --- a/internal/consistency/mtree/merkle.go +++ b/internal/consistency/mtree/merkle.go @@ -110,6 +110,11 @@ type MerkleTreeTask struct { // and OOM the process. MaxDiffRows int64 + // MaxHTMLRows limits how many entries the HTML report shows for each + // node pair. 0 means mtree.diff.max_html_rows from the config, and if + // that is not set either, utils.DefaultMaxHTMLRows. + MaxHTMLRows int64 + DiffResult types.DiffOutput diffMutex sync.Mutex diffRowKeySets map[string]map[string]map[string]struct{} @@ -2401,6 +2406,12 @@ func (m *MerkleTreeTask) DiffMtree() (err error) { if m.MaxDiffRows < 0 { return fmt.Errorf("max_diff_rows must be >= 0, got %d", m.MaxDiffRows) } + if m.MaxHTMLRows < 0 { + return fmt.Errorf("max_html_rows must be >= 0, got %d", m.MaxHTMLRows) + } + if cfg := config.Get(); cfg != nil && cfg.MTree.Diff.MaxHTMLRows < 0 { + return fmt.Errorf("mtree.diff.max_html_rows in the config must be >= 0, got %d", cfg.MTree.Diff.MaxHTMLRows) + } if err = m.UpdateMtree(true); err != nil { // A missing tree already carries a complete, actionable message; @@ -2443,6 +2454,11 @@ func (m *MerkleTreeTask) DiffMtree() (err error) { m.MaxDiffRows = cfg.MTree.Diff.MaxDiffRows } } + if m.MaxHTMLRows == 0 { + if cfg := config.Get(); cfg != nil && cfg.MTree.Diff.MaxHTMLRows > 0 { + m.MaxHTMLRows = cfg.MTree.Diff.MaxHTMLRows + } + } m.diffRowCounts = make(map[string]int64) m.diffLimitWarned = false m.pairCompareErrs = make(map[string]bool) @@ -2615,7 +2631,7 @@ func (m *MerkleTreeTask) DiffMtree() (err error) { logger.Warn("mtree table-diff stopped after reaching max_diff_rows=%d; additional differences may exist", m.MaxDiffRows) } - diffPath, _, writeErr := utils.WriteDiffReport(m.DiffResult, m.Schema, m.Table, m.Output) + diffPath, _, writeErr := utils.WriteDiffReport(m.DiffResult, m.Schema, m.Table, m.Output, m.MaxHTMLRows) if writeErr != nil { return writeErr } diff --git a/internal/consistency/repair/html_plan_e2e_test.go b/internal/consistency/repair/html_plan_e2e_test.go new file mode 100644 index 00000000..5a29051a --- /dev/null +++ b/internal/consistency/repair/html_plan_e2e_test.go @@ -0,0 +1,223 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # 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 repair + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + planner "github.com/pgedge/ace/internal/consistency/repair/plan" + utils "github.com/pgedge/ace/pkg/common" + "github.com/pgedge/ace/pkg/types" +) + +// These tests check the whole path of a repair plan built in the HTML +// report: table-diff writes the JSON and HTML reports, the page script +// (run in Node.js, with no user choices) builds the YAML plan, and the +// repair executor resolves that plan against the full JSON diff. They skip +// when node is not installed. + +// planScript runs buildPlanYaml from the page on the page's embedded data. +// It takes the script and the data out of the page itself, so it tests what +// a browser would run. +const planScript = ` +const fs = require('fs'); +const [htmlPath, outPath] = process.argv.slice(2); +const page = fs.readFileSync(htmlPath, 'utf8'); +const data = page.match(/\n") + return err +} - missingGroups := make([]missingGroup, 0) - if len(missingInB) > 0 { - group := missingGroup{Title: fmt.Sprintf("Missing in %s", nodeB)} - for _, key := range missingInB { - rowA := OrderedMapToMap(rowMapA[key]) - var cells []cell - for _, col := range columns { - valA := stringifyCellValue(rowA[col]) - _, isPK := pkSet[col] - cells = append(cells, cell{ - Column: col, - IsKey: isPK, - NodeAHTML: plainHTML(valA), - NodeBHTML: plainHTML("MISSING"), - NodeBClass: "missing", - HasDiff: false, - }) - } - group.Rows = append(group.Rows, row{ - PKey: displayByKey[key], - Cells: cells, - RowType: "missing_in_b", - HasDiffs: true, - NodeAJSON: buildRowJSONPretty(rowMapA[key], columns), - }) +// writePlanRows writes the htmlPlanRow of every shown row of the pair. first +// tells whether no row has been written yet, across all pairs. +func (p *htmlPairPlan) writePlanRows(w *bufio.Writer, primaryKey []string, first *bool) error { + lists := []struct { + keys []string + rows map[string]types.OrderedMap + planType string + }{ + {p.valueKeys[:p.shownValue], p.rowMapA, planTypeMismatch}, + {p.missingInB[:p.shownMissingB], p.rowMapA, planTypeMissingN2}, + {p.missingInA[:p.shownMissingA], p.rowMapB, planTypeMissingN1}, + } + for _, l := range lists { + for _, key := range l.keys { + if !*first { + w.WriteByte(',') } - missingGroups = append(missingGroups, group) - } - - if len(missingInA) > 0 { - group := missingGroup{Title: fmt.Sprintf("Missing in %s", nodeA)} - for _, key := range missingInA { - rowB := OrderedMapToMap(rowMapB[key]) - var cells []cell - for _, col := range columns { - valB := stringifyCellValue(rowB[col]) - _, isPK := pkSet[col] - cells = append(cells, cell{ - Column: col, - IsKey: isPK, - NodeAHTML: plainHTML("MISSING"), - NodeAClass: "missing", - NodeBHTML: plainHTML(valB), - HasDiff: false, - }) - } - group.Rows = append(group.Rows, row{ - PKey: displayByKey[key], - Cells: cells, - RowType: "missing_in_a", - HasDiffs: true, - }) + *first = false + row := htmlPlanRow{ + Pair: p.pairKey, + NodeA: p.nodeA, + NodeB: p.nodeB, + Key: p.display[key], + Type: l.planType, + PK: pkLiterals(l.rows[key], primaryKey), } - if len(missingGroups) > 0 { - group.DividerBefore = true + if err := writeJSONTo(w, row); err != nil { + return err } - missingGroups = append(missingGroups, group) - } - - pair := pairSection{ - NodeA: nodeA, - NodeB: nodeB, - DiffCount: formatInt64WithCommas(int64(summary.DiffRowsCount[pairKey])), - ValueDiffs: valueDiffs, - Missing: missingGroups, - HasDiffs: len(valueDiffs) > 0 || len(missingGroups) > 0, } - pairs = append(pairs, pair) - } - - report := reportData{ - Summary: summaryData{ - Items: filteredItems, - Breakdown: buildDiffBreakdown(summary.DiffRowsCount), - }, - Pairs: pairs, - RawDiffJSON: template.JS(rawJSON), - CSS: template.CSS(htmlDiffCSS), - JS: template.JS(htmlDiffJS), } + return nil +} - tmpl, err := template.New("tableDiffReport").Parse(htmlDiffTemplate) +func writeJSONTo(w *bufio.Writer, v any) error { + b, err := json.Marshal(v) if err != nil { - return "", fmt.Errorf("failed to parse HTML template: %w", err) - } - - var buf bytes.Buffer - if err := tmpl.Execute(&buf, report); err != nil { - return "", fmt.Errorf("failed to render HTML diff report: %w", err) - } - - if err := WriteFileSecure(htmlPath, buf.Bytes()); err != nil { - return "", fmt.Errorf("failed to write HTML diff report: %w", err) + return err } - - return htmlPath, nil + _, err = w.Write(b) + return err } func highlightDifference(a, b string) (template.HTML, template.HTML) { @@ -457,8 +894,9 @@ func buildDiffBreakdown(diffCounts map[string]int) []htmlPairCount { } // buildRowKey returns the identity a row is matched by across the two nodes. -// It falls back to the row's position when there is no usable primary key, -// which pairs nothing but at least keeps distinct rows distinct. +// It falls back to the row's position when there is no usable primary key. +// That keeps distinct rows of one node distinct, but it pairs rows of the two +// nodes by position only: __row_0 on A meets __row_0 on B, whatever they hold. func buildRowKey(row types.OrderedMap, primaryKey []string, index int) string { if len(primaryKey) == 0 { return fmt.Sprintf("__row_%d", index) @@ -726,10 +1164,16 @@ func comparePKKey(a, b string) int { } } +// comparePKComponent orders numbers by value and before all other strings, +// and other strings byte-wise. The order must be total: the HTML report shows +// a prefix of the sorted rows, and with an order that is not transitive (as +// when a number and a string compared as strings, so that "1a" < "9" < "10" +// but "10" < "1a") the prefix changed from one run to the next. func comparePKComponent(a, b string) int { numA, okA := parseNumeric(a) numB, okB := parseNumeric(b) - if okA && okB { + switch { + case okA && okB: switch { case numA < numB: return -1 @@ -738,14 +1182,12 @@ func comparePKComponent(a, b string) int { default: return 0 } - } - if a < b { + case okA: return -1 - } - if a > b { + case okB: return 1 } - return 0 + return strings.Compare(a, b) } func parseNumeric(val string) (float64, bool) { @@ -753,7 +1195,9 @@ func parseNumeric(val string) (float64, bool) { return 0, false } num, err := strconv.ParseFloat(val, 64) - if err != nil { + // NaN is not equal to anything, itself included, and would break the + // order; treat it and the infinities as text. + if err != nil || math.IsNaN(num) || math.IsInf(num, 0) { return 0, false } return num, true diff --git a/pkg/common/html_reporter_test.go b/pkg/common/html_reporter_test.go new file mode 100644 index 00000000..589e79c1 --- /dev/null +++ b/pkg/common/html_reporter_test.go @@ -0,0 +1,570 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # 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 common + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/pgedge/ace/pkg/types" +) + +// htmlTestDiff builds a two-node diff with the given number of value +// differences, rows missing on n2 and rows missing on n1. Primary keys are +// 1..N in that order, so the report order is easy to predict. +func htmlTestDiff(valueDiffs, missingOnB, missingOnA int, note string) types.DiffOutput { + row := func(id int, val string) types.OrderedMap { + return types.OrderedMap{ + {Key: "id", Value: id}, + {Key: "val", Value: val}, + {Key: "note", Value: note}, + } + } + var a, b []types.OrderedMap + id := 0 + for i := 0; i < valueDiffs; i++ { + id++ + a = append(a, row(id, "on-n1")) + b = append(b, row(id, "on-n2")) + } + for i := 0; i < missingOnB; i++ { + id++ + a = append(a, row(id, "only-n1")) + } + for i := 0; i < missingOnA; i++ { + id++ + b = append(b, row(id, "only-n2")) + } + return types.DiffOutput{ + NodeDiffs: map[string]types.DiffByNodePair{ + "n1/n2": {Rows: map[string][]types.OrderedMap{"n1": a, "n2": b}}, + }, + Summary: types.DiffSummary{ + Schema: "public", + Table: "t", + Nodes: []string{"n1", "n2"}, + PrimaryKey: []string{"id"}, + DiffRowsCount: map[string]int{"n1/n2": valueDiffs + missingOnB + missingOnA}, + }, + } +} + +type htmlTestData struct { + Summary types.DiffSummary `json:"summary"` + HTMLReport htmlReportInfo `json:"report_info"` + Rows []htmlPlanRow `json:"rows"` +} + +// typeCounts counts the embedded plan rows of each type for one pair. +func (d htmlTestData) typeCounts(pair string) map[string]int { + c := map[string]int{} + for _, r := range d.Rows { + if r.Pair == pair { + c[r.Type]++ + } + } + return c +} + +var diffDataRe = regexp.MustCompile(`(?s)`) + +// renderHTMLTestReport writes the report and returns the page and the parsed +// embedded diff data. +func renderHTMLTestReport(t *testing.T, diff types.DiffOutput, maxRows int64) (string, htmlTestData) { + t.Helper() + jsonPath := filepath.Join(t.TempDir(), "public_t_diffs-20260101000000.json") + htmlPath, err := writeHTMLDiffReport(diff, jsonPath, maxRows) + if err != nil { + t.Fatalf("writeHTMLDiffReport: %v", err) + } + raw, err := os.ReadFile(htmlPath) + if err != nil { + t.Fatalf("read report: %v", err) + } + page := string(raw) + if !strings.HasSuffix(strings.TrimSpace(page), "") { + t.Fatal("report does not end with ") + } + + m := diffDataRe.FindStringSubmatch(page) + if m == nil { + t.Fatal("report has no diff-data script") + } + var data htmlTestData + if err := json.Unmarshal([]byte(m[1]), &data); err != nil { + t.Fatalf("embedded diff data is not valid JSON: %v", err) + } + return page, data +} + +// countRows counts rendered rows of one type ("value_diff", "missing_in_a", +// "missing_in_b"). Every rendered row has exactly one select checkbox. +func countRows(page, rowType string) int { + re := regexp.MustCompile(`class="row-select" data-pk="[^"]*" data-type="` + rowType + `"`) + return len(re.FindAllStringIndex(page, -1)) +} + +func TestHTMLReportNotTruncated(t *testing.T) { + page, data := renderHTMLTestReport(t, htmlTestDiff(4, 2, 1, "x"), 0) + + // Look for the elements, not the bare class names: the embedded CSS + // always has those. + for _, marker := range []string{`class="truncation-banner"`, `class="truncation-note"`, `class="truncation-row"`, "Rows Shown in Report"} { + if strings.Contains(page, marker) { + t.Errorf("report that shows every entry contains %q", marker) + } + } + if !strings.Contains(page, ">7 rows<") { + t.Error("section pill does not show the plain row count") + } + if got := countRows(page, "value_diff"); got != 4 { + t.Errorf("value rows: got %d, want 4", got) + } + if got := countRows(page, "missing_in_b"); got != 2 { + t.Errorf("missing_in_b rows: got %d, want 2", got) + } + if got := countRows(page, "missing_in_a"); got != 1 { + t.Errorf("missing_in_a rows: got %d, want 1", got) + } + + if data.HTMLReport.Truncated { + t.Error("report_info.truncated is true for a complete report") + } + if data.HTMLReport.MaxRows != DefaultMaxHTMLRows { + t.Errorf("max_html_rows: got %d, want the default %d", data.HTMLReport.MaxRows, DefaultMaxHTMLRows) + } + want := map[string]int{"row_mismatch": 4, "missing_on_n2": 2, "missing_on_n1": 1} + if got := data.typeCounts("n1/n2"); fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("embedded rows: got %v, want %v", got, want) + } + if data.Summary.Table != "t" || len(data.Summary.PrimaryKey) != 1 { + t.Errorf("embedded summary is wrong: %+v", data.Summary) + } +} + +// The limit takes entries in report order: value differences first, then +// rows missing on n2, then rows missing on n1. +func TestHTMLReportTruncated(t *testing.T) { + // 25 value differences, 10 missing on n2, 5 missing on n1; limit 30. + page, data := renderHTMLTestReport(t, htmlTestDiff(25, 10, 5, "x"), 30) + + if got := countRows(page, "value_diff"); got != 25 { + t.Errorf("value rows: got %d, want 25", got) + } + if got := countRows(page, "missing_in_b"); got != 5 { + t.Errorf("missing_in_b rows: got %d, want 5", got) + } + if got := countRows(page, "missing_in_a"); got != 0 { + t.Errorf("missing_in_a rows: got %d, want 0", got) + } + if !strings.Contains(page, ">Missing in n2<") { + t.Error("group of rows missing on n2 is not shown") + } + if strings.Contains(page, ">Missing in n1<") { + t.Error("group header for rows missing on n1 is shown, but none of its rows are") + } + + for _, want := range []string{ + "This report shows 30 of 40 rows", + "public_t_diffs-20260101000000.json", + "Its default action is skip", + "Rows Shown in Report", + "30 of 40 rows shown", + "Not shown: 5 missing in n2, 5 missing in n1.", + "table-repair skips the other rows", + "they stay different", + "--max-html-rows", + "10 more rows are not shown (5 missing in n2, 5 missing in n1)", + "Default: insert from n1", + `data-truncated="true"`, + } { + if !strings.Contains(page, want) { + t.Errorf("truncated report does not contain %q", want) + } + } + + // The page script builds repair plans from the embedded data, so it + // must hold exactly the rows on the page and say that it is partial. + want := map[string]int{"row_mismatch": 25, "missing_on_n2": 5} + if got := data.typeCounts("n1/n2"); fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("embedded rows: got %v, want %v", got, want) + } + info := data.HTMLReport + if !info.Truncated || info.MaxRows != 30 || info.DiffFile != "public_t_diffs-20260101000000.json" { + t.Errorf("report_info: got %+v", info) + } + if len(info.Pairs) != 1 || info.Pairs[0] != (htmlPairInfo{Pair: "n1/n2", Shown: 30, Total: 40}) { + t.Errorf("report_info.pairs: got %+v", info.Pairs) + } +} + +func TestHTMLReportTruncatedInsideValueDiffs(t *testing.T) { + page, data := renderHTMLTestReport(t, htmlTestDiff(25, 10, 5, "x"), 20) + + if got := countRows(page, "value_diff"); got != 20 { + t.Errorf("value rows: got %d, want 20", got) + } + if strings.Contains(page, ">Missing Rows<") { + t.Error("missing rows separator is shown, but no missing row is") + } + want := "20 more rows are not shown (5 value differences, 10 missing in n2, 5 missing in n1)" + if !strings.Contains(page, want) { + t.Errorf("footer does not contain %q", want) + } + + // The shown rows are the first 20 primary keys. + if len(data.Rows) != 20 { + t.Fatalf("embedded rows: got %d, want 20", len(data.Rows)) + } + for i, r := range data.Rows { + if r.PK[0] != fmt.Sprint(i+1) { + t.Fatalf("embedded row %d has pk %v, want %d", i, r.PK, i+1) + } + } +} + +// Primary key values go into the embedded JSON inside a