fix(#461): release configuration write lock around locator.configure()#466
Merged
Conversation
Copilot created this pull request from a session on behalf of
eleanorjboyd
May 27, 2026 17:15
View session
Performance Report (Linux) ✅
Legend
|
Performance Report (macOS)
Legend
|
Performance Report (Windows) ➖
Legend
|
eleanorjboyd
changed the title
Test Coverage Report (Linux)
Coverage increased! Great work! |
Test Coverage Report (Windows)
Coverage increased! Great work! |
eleanorjboyd
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes a performance regression where configure held the configuration RwLock write guard across potentially slow locator.configure() I/O, which could block refresh-side configuration.read() calls and inflate pet.refresh latency (Fixes #461). The change restores the pre-#416 concurrency behavior while preserving the “publish new generation only after all locators are configured” invariant.
Changes:
- Add a
configure_in_progressmutex to serialize overlappingconfigureRPCs without using the configurationRwLockas the serialization mechanism. - Refactor
apply_configure_optionsinto “prepare (snapshot) → configure locators (no config lock) → publish (short write lock)” phases. - Update existing configure test expectations and add new tests for refresh non-blocking behavior, configure serialization, and panic rollback behavior.
Show a summary per file
| File | Description |
|---|---|
| crates/pet/src/jsonrpc.rs | Introduces configure_in_progress and restructures apply_configure_options to avoid holding configuration.write() during locator configuration; adds/updates concurrency regression tests for #461. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 3
rchiodo
reviewed
May 27, 2026
| @@ -0,0 +1,569 @@ | |||
| # PET Regression Investigation — Response to May 25 Report | |||
There was a problem hiding this comment.
@copilot -569
[Architect] This 569-line dated investigation document references specific insider build versions (1.33.2026051401, etc.), SQM IDs, and time-sensitive action items ("Target: end of week"). It will become stale quickly. Consider moving this to a GitHub issue, wiki page, or discussion thread where it can be updated/closed — checked-in docs/ files tend to remain as outdated artifacts.
[verified]
Contributor
Author
There was a problem hiding this comment.
Removed the dated investigation document from docs/ in commit ebf4dfa.
dmitrivMS
approved these changes
May 27, 2026
eleanorjboyd
added a commit
that referenced
this pull request
May 27, 2026
## Summary Run the Homebrew setup commands in the `homebrew/brew` container as the `linuxbrew` user instead of root, while discovering the `brew` executable from the container PATH at runtime. The container job still runs as root so GitHub Actions checkout/tooling can write to the mounted workspace, but recent `homebrew/brew` images have been brittle when `brew install` itself runs as root. PR #466 is currently failing before Rust setup/tests with Homebrew crashing while pouring the `ca-certificates` bottle: ```text undefined method '[]' for nil /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/utils/bottles.rb:127:in 'Utils::Bottles.load_tab' ``` This PR keeps the existing container shape but switches the install step to: ```bash brew_bin="$(command -v brew)" su linuxbrew -c "${brew_bin} update || true" su linuxbrew -c "${brew_bin} install python@3.12 python@3.11" ``` That avoids hardcoding the Linuxbrew prefix while still avoiding root-owned Homebrew install behavior. ## Validation This is a CI-only workaround. The useful validation is the `ci-homebrew-container` Actions job on this PR.
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.
Fixes #461