-
Notifications
You must be signed in to change notification settings - Fork 310
sync_diff_inspector: avoid full-table scan for limit iterator progress #12727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,3 +106,33 @@ func TestAllSuccess(t *testing.T) { | |
| "You can view the comparison details through './output_dir/sync_diff_inspector.log'\n\n", | ||
| ) | ||
| } | ||
|
|
||
| // TestProgressEstimateClampAndMarker verifies that an underestimated, not-yet | ||
| // finalized total (as produced by the limit iterator) never overflows the bar | ||
| // and is rendered with the "~" estimation marker. Before clamping, driving the | ||
| // progress past the total made strings.Repeat receive a negative count and | ||
| // panic the serve goroutine; the test completing at all proves the clamp. | ||
| func TestProgressEstimateClampAndMarker(t *testing.T) { | ||
| p := newTableProgressPrinter(1, 0) | ||
| p.RegisterTable("t", false, false, common.AllTableExistFlag) | ||
| // Seed with a deliberate under-estimate (2) that is not finalized. | ||
| p.StartTable("t", 2, false) | ||
|
|
||
| buffer := new(bytes.Buffer) | ||
| p.SetOutput(buffer) | ||
|
|
||
| // Drive progress well past the estimate without finalizing the total. | ||
| for i := 0; i < 10; i++ { | ||
| p.Inc("t") | ||
| } | ||
| time.Sleep(300 * time.Millisecond) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| p.Close() | ||
|
|
||
| out := buffer.String() | ||
| // The figures are marked as estimated while the total keeps growing. | ||
| require.Contains(t, out, "~") | ||
| // The percentage is clamped to 100% and never overflows it. | ||
| require.Contains(t, out, "100%") | ||
| require.NotContains(t, out, "101%") | ||
| require.NotContains(t, out, "200%") | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, this checks
totalStopUpdatefor all tables intpp.tableMap. However, tables that are registered but not yet started also reside intpp.tableMapand havetotalStopUpdateset tofalseby default. This causes the progress bar to display the estimation marker~even when using exact splitters (like bucket or random) until the very last table starts. We should only checktotalStopUpdatefor tables that are actually active (i.e., intableStatePrestartortableStateComparingstate).