Update table-diff.md with max_diff_rows information#129
Conversation
Added a note about the max_diff_rows setting in table-diff documentation, explaining its global cap across node pairs and implications for reported differences.
📝 WalkthroughWalkthroughAdds 10 lines to Changestable-diff max_diff_rows documentation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/commands/diff/table-diff.md (1)
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace intensifier "very large" with stronger wording.
The style checker flags "very large" as a weak intensifier. Consider "large" or a specific quantitative description if appropriate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/commands/diff/table-diff.md` at line 90, Replace the weak intensifier in the sentence starting with “When the number of differences between nodes...” in the table diff documentation with stronger, clearer wording such as “large” or a more specific quantitative description; keep the rest of the sentence unchanged and ensure the revised phrasing matches the doc style used in this section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/commands/diff/table-diff.md`:
- Around line 83-87: The `table-diff` docs currently describe `max_diff_rows` as
a global limit across all node pairs, but the code paths in `table-diff` are
pair-scoped. Update the wording in the `table-diff` documentation to say the cap
is enforced per node pair (for example, n1/n2 and n1/n3 each track their own
limit) and that each pair can independently stop at `max_diff_rows`. Keep the
warning text aligned with this behavior so readers understand the limit is not
shared across pairs.
---
Nitpick comments:
In `@docs/commands/diff/table-diff.md`:
- Line 90: Replace the weak intensifier in the sentence starting with “When the
number of differences between nodes...” in the table diff documentation with
stronger, clearer wording such as “large” or a more specific quantitative
description; keep the rest of the sentence unchanged and ensure the revised
phrasing matches the doc style used in this section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d3ebbd4-2e0b-4c4b-a3d3-9dc539378ff7
📒 Files selected for processing (1)
docs/commands/diff/table-diff.md
| 5. The `max_diff_rows` setting caps the number of differing rows that `table-diff` collects in a single run. | ||
| - The cap applies globally across **all node pairs combined**, not to each pair individually. On a three-node cluster that diffs n1/n2 and n1/n3 at the same time, both pairs share the single cap. As a result, each pair can report fewer | ||
| differences than the table actually contains. | ||
| - The `table-diff stopped after reaching max_diff_rows` warning indicates that the run hit the cap. Treat the reported result as partial whenever the warning appears. Re-run `table-diff` after each repair pass, and continue until the command reports `TABLES MATCH`. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚖️ Poor tradeoff
Documentation incorrectly states max_diff_rows is a global cap across all node pairs.
The implementation and tests show max_diff_rows is enforced per node pair, not globally across all pairs. The pairDiffRows map in internal/consistency/diff/table_diff.go stores separate atomic counters keyed by pair, and shouldStopPair checks the limit independently for each pair. The integration test TestTableDiffMaxDiffRowsPerPair explicitly validates this: with max_diff_rows=14 and two pairs each differing by 10 rows, both pairs fully report their diffs and DiffRowLimitReached remains false—behavior that would be impossible under a global cap.
Please correct the documentation to state that the cap applies per node pair (e.g., n1/n2, n1/n3), not globally across all pairs combined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/commands/diff/table-diff.md` around lines 83 - 87, The `table-diff` docs
currently describe `max_diff_rows` as a global limit across all node pairs, but
the code paths in `table-diff` are pair-scoped. Update the wording in the
`table-diff` documentation to say the cap is enforced per node pair (for
example, n1/n2 and n1/n3 each track their own limit) and that each pair can
independently stop at `max_diff_rows`. Keep the warning text aligned with this
behavior so readers understand the limit is not shared across pairs.
Correct formatting and indentation.
Added a note about the max_diff_rows setting in table-diff documentation, explaining its global cap across node pairs and implications for reported differences.