Page entity-store scans in startup migrations instead of one unbounded read#872
Conversation
MigrateEntityStore and MigrateShortIds each read the entire entity keyspace with a single Get(WithPrefix()). On a large or not-recently-compacted etcd that single unbounded request can stall, and because these run early in coordinator startup it can block the edge from coming up. Add listEntitiesPaged, which reads the prefix in bounded pages (WithLimit + a key cursor in ascending key order), and use it in both migrations. Per-entity processing is unchanged; only the read is paged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
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 |
#872 added listEntitiesPaged to bound two startup scans, but the principled home is one primitive every full-keyspace read shares. Rename it scanPaged, give it keys-only and page-size options plus an EtcdStore method, and route the three reads #872 missed through it: the short-id unique-key set, ListAllEntityIDs, and reindex's stale-cleanup scan. No startup scan open-codes Get(WithPrefix()) now. Also collapse the reindex timeout nesting from #871. checkAndReindex's inner 5-min budget always clamped to whatever was left of the 2-min startup ceiling, so it was dead. Drop it and let reindex share the single 2-min bound that keeps the coordinator off the edge listener's back; an unfinished reindex retries on the next boot.
2 participants
What
MigrateEntityStoreandMigrateShortIdseach read the entire entity keyspace with a singleclient.Get(ctx, prefix, clientv3.WithPrefix()). This adds a smalllistEntitiesPagedhelper that reads the prefix in bounded pages (WithLimit+ a key cursor, ascending key order) and uses it in both migrations. Per-entity processing is unchanged — only the read is paged.Why
A single unbounded prefix read loads the whole keyspace (keys and values) in one RPC. On a large or not-recently-compacted store that request can stall, and because these migrations run early in coordinator startup, a stall blocks the edge listener from coming up. We hit exactly this on a self-hosted node — the migration's full read hung for hours after a restart, leaving every app unreachable (more detail in the companion PR #871). Paging keeps each request bounded and predictable regardless of store size.
As with #871, we may be missing context here, so this is offered as "what worked for us" rather than a prescription.
Notes
go build,go vet, andgofmtare clean. Thepkg/entityintegration tests need the dev etcd environment (make dev) and weren't run locally.MigrateShortIdsis keys-only (bounded payload), so it's left as-is — glad to page it too if you'd prefer consistency.