Add support for System.WaitForConfig and WaitForClientConfig#324
Merged
Add support for System.WaitForConfig and WaitForClientConfig#324
Conversation
This adds push-based notification for config checks using a close-and-replace channel pattern on inboundManager. A single configCh is closed and replaced on every rebuildConfig call, waking all waiters simultaneously. A separate stopCh (closed via sync.Once) unblocks waiters when the system stops. New API on System: WaitForConfig(ctx, cond) — waits on known-peer config WaitForClientConfig(ctx, cond) — waits on client-peer config Both return nil when cond is satisfied, ctx.Err() on cancellation, and ErrStopped when the system is stopped before the condition is met. The condition is checked immediately so they return without blocking if already satisfied. Add ErrStopped sentinel error to errors.go. Add six subtests under TestWaitForConfig covering: condition already met, condition met after connect, context cancelled, system stopped, concurrent waiters, and client config variant. Document WaitForConfig and WaitForClientConfig in the user guide. Fixes #323
…lpers Remove the polling WaitForConfigCondition helper from testing_shared.go. Add waitForKnownConfig and waitForClientConfig to inboundManager, each accepting func(Configuration) bool matching the public System API shape. Simplify System.WaitForConfig and System.WaitForClientConfig to delegate directly to the new typed helpers instead of inlining raw field access. Add mustWaitForConfig and mustWaitForClientConfig test helpers in inbound_manager_test.go that call the typed methods, eliminating the locking-regiment leak where callers had to read internal fields directly. Replace all WaitForConfigCondition call sites in system_test.go with the public System.WaitForConfig / System.WaitForClientConfig API. This was mainly done to make sure that the new APIs work well also for tests.
Extend the storage example to demonstrate WaitForConfig in a real multi-process cluster scenario. This commit also moves Gorums specific code to server.go keeping main.go for scaffolding. Three modes are now supported from a single binary: - No flags: spins up an in-process four-node cluster (runLocalCluster) and opens the client REPL directly against it. - -cluster -addrs <addr,...>: spawns one server subprocess per address using os/exec, prints connection instructions, and stops all children on Ctrl-C. - -serve -addrs <myaddr>[,<peer>...]: runs as a single leaf-node server, used by -cluster internally or for manual deployments. Other cleanups: splitAddrs called once in main; runClient errors are returned instead of calling log.Fatal; helper functions peerConfig, registerAndServe, and runLocalCluster are extracted to keep runServer focused; Added Makefile start target to use the new -cluster flag.
Contributor
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go |  |
Apr 12, 2026 5:16p.m. | Review ↗ |
| Shell | |
Apr 12, 2026 5:16p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds notification-based configuration waiting APIs to System, enabling callers (and examples/tests) to block until a desired known-peer or client-peer configuration state is reached without polling.
Changes:
- Introduces
System.WaitForConfig/System.WaitForClientConfigplusErrStoppedfor shutdown-interrupted waits. - Implements push-based config-change notifications in
inboundManagerto unblock waiters on rebuilds. - Updates tests, user guide, and
examples/storageto use and demonstrate the new wait APIs (including a multi-process cluster runner).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testing_shared.go | Removes the old polling-based WaitForConfigCondition helper. |
| system.go | Adds WaitForConfig / WaitForClientConfig; ensures Stop() unblocks waiters. |
| system_test.go | Migrates config-wait logic to the new public APIs and adds dedicated wait tests. |
| inbound_manager.go | Adds broadcast channels + wait helpers to support notification-based waiting. |
| inbound_manager_test.go | Replaces polling waits with new wait helpers for known/client configs. |
| errors.go | Adds ErrStopped sentinel error for interrupted waits. |
| doc/user-guide.md | Documents the new wait APIs, usage patterns, and return values. |
| examples/storage/server.go | Demonstrates WaitForConfig and adds deterministic peer ID setup for multi-node runs. |
| examples/storage/main.go | Adds new CLI modes and address parsing helper. |
| examples/storage/cluster.go | New helper to spawn multi-process storage clusters. |
| examples/storage/client.go | Converts client entrypoint to return errors instead of exiting internally. |
| examples/Makefile | Adds make start to run a local 3-node storage cluster. |
| .vscode/gorums.txt | Adds a new word to the local dictionary list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds notification-based config waiting to
System, so callers can wait for a desired configuration state without pollingConfig()in a loop.The new API adds:
WaitForConfig(ctx, cond)WaitForClientConfig(ctx, cond)Both methods check the condition immediately, return when the condition is satisfied, return
ctx.Err()on cancellation, and returnErrStoppedif the system stops before the condition is met.Internally, this is implemented with push-based notifications in
inboundManager, which broadcasts configuration rebuilds to waiting goroutines as soon as the config changes.Changes
System.WaitForConfigandSystem.WaitForClientConfigErrStoppedfor interrupted waits during shutdownexamples/storageto demonstrateWaitForConfigin a real multi-process cluster setupTesting
Fixes #323