feat(github): render live row-copy progress on sharded table lines - #1191
feat(github): render live row-copy progress on sharded table lines#1191aparajon wants to merge 3 commits into
Conversation
a58e873 to
64c36fa
Compare
62cf08c to
01658d0
Compare
e110697 to
f3ecde5
Compare
cf7ded3 to
d0aa6b4
Compare
A copying table on a sharded apply now shows the same progress bar, rows line, and ETA a single-target apply shows, aggregated at render time from the per-shard task rows: rows sum across the shards that have reported, the ETA is the slowest shard's, and the percentage comes from the summed rows rather than averaged shard percents. The honesty guards carry over — a zero copied count renders "Starting copy..." and an exceeded estimate renders the activity bar with the "so far" count instead of a false percentage. Rows only qualify a table in the Running phase, so checksum and cutover phases keep their state phrases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d0aa6b4 to
e03e49e
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances sharded apply PR-comment UX by aggregating per-shard row-copy metrics (rows copied/total + ETA) into the per-table rollup line, so operators can see live copy progress at the table level instead of only a generic state phrase.
Changes:
- Aggregate
RowsCopied,RowsTotal, andETASecondsacross shard tasks when building sharded apply template data. - Render a per-table progress bar + rows/ETA line for actively copying sharded tables (keeping non-copy phases on state phrases).
- Add/adjust previews and tests to cover the new rendering and edge cases (starting copy, estimate exceeded, non-copy phases).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TEMPLATES.md | Updates rendered sharded-apply examples to show table-level copy progress (bar + rows/ETA) and adjusts cancelled glyph in the example output. |
| pkg/webhook/templates/sharded_apply.go | Adds aggregated row-copy fields to ShardedTableStatus and renders a table-level progress bar + rows/ETA when copying. |
| pkg/webhook/templates/sharded_apply_test.go | Adds template-level tests covering progress rendering, starting-copy, estimate-exceeded, and gating rows to the copy phase. |
| pkg/webhook/templates/preview_sharded.go | Updates sharded preview fixtures to include row-copy metrics so previews match the new renderer. |
| pkg/webhook/sharded_apply.go | Aggregates shard task progress into per-table rollups (sum rows, max ETA) at render-data build time. |
| pkg/webhook/sharded_apply_test.go | Updates/extends builder tests to assert row/ETA aggregation behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A large table early in its copy has rows flowing but an integer fraction that rounds down to zero, which rendered the starting indicator as if no rows had been copied. Route the percent through ui.RowCopyDisplayPercent, matching the single-target renderer, so the starting form is reserved for a copy that has not reported rows yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
morgo
left a comment
There was a problem hiding this comment.
🤖 Approved on Morgan's behalf by his AI agent.
Comment-rendering only, 37/37 green, and it reuses the single-deployment honesty guards (EstimateExceeded → "Finalizing copy", pct == 0 → "Starting copy...") rather than reinventing them. The division is safe — t.RowsTotal > 0 gates the whole branch — and shardTaskProgress replacing best wholesale rather than field-by-field is the right call, since it can't splice one task's status onto another's row counts.
Approving because this is strictly better than showing no progress at all. But two of the aggregated figures claim more than they can deliver once a rollout spans more than one dispatch wave, and I'd rather flag them now than have someone file them as bugs later.
1. The percentage regresses as waves start. RowsTotal sums only the shards that have reported. A 4-shard table with wave 1 in flight renders 500k / 1M = 50%; when wave 2 starts and adds its shard's total, the same table renders 500k / 2M = 25%. The bar goes backwards, in a comment that's edited in place, while the operator is watching. The doc comment frames the growing total as a feature — "live, not a plan-time promise" — which is a fair description of the mechanism but doesn't acknowledge that the rendered consequence is a progress bar that visibly loses ground.
2. The ETA is the slowest started shard, not the slowest shard. Unstarted shards contribute etaSeconds: 0, so max skips them entirely. During wave 1 of 4, the line reads as the table's ETA but is really wave 1's ETA — it will systematically under-report, and it under-reports most severely at the start, which is when people read it. "ETA 3 minutes" on a table with three waves still to dispatch is the kind of thing that costs the tool credibility.
Neither is hard to soften if you want them: denominating the percentage over all shards (treating unreported shards as unknown rather than zero), or suppressing the ETA line until every shard has reported, would both make the number mean what the label says. A cheaper option is to keep the figures and qualify the label while waves are outstanding — e.g. "ETA 3m (shards still dispatching)" — so the number stays useful without overclaiming.
Nit: the preview fixture bumps -40 from 45% to 62% alongside the new row figures. Worth a sanity check that the fixture's shard percent and the new RowsCopied/RowsTotal (914707/1466232 ≈ 62%) are meant to agree — they do here, which is a nice touch, but nothing enforces it, so a future edit to one can silently desync the preview from what the real renderer would produce.
The summed rows and slowest-shard ETA cover only the shards whose engine has reported figures. Before every dispatch wave has started, the plain rows line presented a wave's fraction as the whole table's: the bar lost ground when the next wave's total joined the denominator, and the ETA read as the table's while describing only the started shards. Carry the reporting-shard count on the rollup and, while it trails the shard count, render the coverage on the rows line and the ETA as a floor. The preview fixture's shard percent is now derived from its row figures so the two cannot drift apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Why this matters
A copying table on a sharded apply shows only a state word, while the real copy progress — rows copied, totals, ETA — already sits in the per-shard task rows the driver writes through on every poll. The operator watching the PR gets less information than the system has. This PR puts that live progress on the table line, rendered the same way a single-target apply renders it.
What it does
The table line aggregates its shard tasks at render time — the template stays dumb and the per-shard write path is untouched:
Three safety properties worth calling out:
Because the aggregate reads durable task rows, any pod rendering the comment shows the same numbers — there is no in-memory progress state to lose across a lease handover.
Apply In Progress
Schema Change Status — Production
Database:
cdb_resolute| Type:Strata| Apply ID:apply-a1b2c3d4e5f6Applied by @jackjackbits at 2026-01-01 00:00:00 UTC
Status: In Progress — 0 of 1 change applied
Shards: 1 running table copy, 3 waiting for -40
Keyspace
cdb_resolute_shardedmutes: 🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦⬜⬜⬜⬜⬜⬜⬜⬜ 62%└ shards: ◐ -40 62% · ⏳ 40-80 · ⏳ 80-c0 · ⏳ c0-
Last updated:2026-01-01 00:00:00 UTC (2026-01-01 00:00:00 UTC)
Apply Across Multiple Keyspaces
Schema Change Status — Production
Database:
cdb_resolute| Type:Strata| Apply ID:apply-a1b2c3d4e5f6Applied by @jackjackbits at 2026-01-01 00:00:00 UTC
Status: In Progress — 1 of 4 changes applied
Shards: 1 completed, 1 running table copy, 4 waiting for cdb_resolute_lookup/-
Keyspace
cdb_resoluteoutcomes: ✅ CompleteKeyspace
cdb_resolute_lookupoutcomes_lookup: 🟦🟦🟦🟦🟦⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜ 27%Keyspace
cdb_resolute_shardedmutes: ⏳ QueuedVSchema
cdb_resolute_sharded: PendingLast updated:2026-01-01 00:00:00 UTC (2026-01-01 00:00:00 UTC)
How it moves us toward the northstar
This closes the per-shard progress loop for sharded targets: the durable per-shard write-through now reaches the operator on the PR as honest table-level copy progress, completing the comment redesign this series set out (#1188–#1190).
Opened by Claude (Fable 5).