Bound startup entity maintenance with a timeout so a slow etcd can't block the edge#871
Conversation
The coordinator's startup maintenance phase (entity-format migration, short-id backfill, reindex) scans the entire entity store. Each step reads with an unbounded Get(WithPrefix()); on a large or not-recently-compacted etcd that read can stall, blocking the coordinator -- and therefore the edge listener -- indefinitely. Each step already treats failure as non-fatal (warn and proceed), so wrap the phase in a 2-minute context timeout. A slow store now fails fast and startup continues, deferring the work to a later boot once compaction catches up, instead of hanging the edge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIn 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 |
phinze
left a comment
phinze
left a comment
There was a problem hiding this comment.
Oof a cluster stuck for 7.5h on boot is no fun - sorry you hit this!
Both this and #872 look sound and I'm merging them as-is. I've filed MIR-1272 with a few follow up details I'll pick up rather than bouncing nits back to you.
Really appreciate you for coming around with not just a bug report but a full on diagnosis and fix... thank you for taking the extra time to make our job easier ❤️
#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
The coordinator's startup maintenance phase — entity-format migration (
MigrateEntityStore), short-id backfill (MigrateShortIds), and reindex (checkAndReindex) — scans the entire entity store. Each step reads with an unboundedclient.Get(ctx, prefix, clientv3.WithPrefix()). On a large or not-recently-compacted etcd, that single request can stall, and because it runs before the edge listener is up, the node stays dark until it returns.This wraps the maintenance phase in a
context.WithTimeout(2 min). Each step already treats failure as non-fatal (logs a warning and proceeds), so a timeout turns "hang forever" into "fail fast, continue startup, and retry on the next boot once compaction catches up."Why
We hit this on a self-hosted single-node cluster (v0.10.0). After a routine restart the coordinator parked in
starting entity migrationand never bound:80/:443— every app on the box was unreachable for ~7.5 h, while the app containers themselves kept running fine. etcd had accumulated a lot of MVCC history (its scheduled compaction kept getting interrupted by the resulting restart loop), and the unbounded read stalled indefinitely. Small /--limitreads stayed instant; only the full prefix read hung. Manually compacting + restarting cleared it, and the migration then completed in ~50 ms.We may well be missing context on why the read is structured this way, so please treat this as "here's what we saw" rather than a prescription — happy to adjust.
Notes
go build,go vet, andgofmtare clean. Thepkg/entityintegration tests require the dev etcd environment (make dev), so they weren't run locally.