refactor(validator): share one MirrorClient across the OSS scoring round#1562
Closed
ebios-star wants to merge 1 commit into
Closed
refactor(validator): share one MirrorClient across the OSS scoring round#1562ebios-star wants to merge 1 commit into
ebios-star wants to merge 1 commit into
Conversation
get_rewards opened a fresh MirrorClient (hence a new requests.Session) for every UID inside the per-miner loop, throwing away the mirror.gittensor.io connection pool between miners and re-handshaking TLS on each iteration. Hoist a single MirrorClient to the round level and thread it through evaluate_miners_pull_requests, matching how issue discovery already passes one client through every miner (scan.py). evaluate_miners_pull_requests now takes an optional mirror_client and only owns/closes a client when it created one itself, so standalone/test calls keep their previous behavior. Behavior-preserving: MirrorClient holds no per-miner state, so sharing the instance only reuses the connection pool. Adds two tests pinning the client ownership contract (injected client reused, standalone client closed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@anderdc @LandynDev whenever you have a moment — small, behavior-preserving refactor. |
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
get_rewardsopened a freshMirrorClient— and therefore a brand-newrequests.Session— for every UID inside the per-miner loop:So a full scoring round builds and tears down one HTTPS session per miner against
mirror.gittensor.io, discarding the connection pool between miners and re-doing the TLS handshake on every iteration. Connection reuse also matters under the mirror's Cloudflare rate limit (50 req / 10s / IP).This PR hoists a single
MirrorClientto the round level inget_rewardsand threads it throughevaluate_miners_pull_requests. This mirrors the pattern the issue-discovery path already uses — it builds one client and passes it through every miner (issue_discovery/scan.py:156). The OSS-scoring path is now consistent with it.evaluate_miners_pull_requestsgains an optionalmirror_clientparameter and only owns/closes a client when it created one itself; a caller-supplied client stays open for the next miner. Standalone and test calls (which pass no client) keep their previous create-and-close behavior, so there is no new session leak on that path.Why it's behavior-preserving
MirrorClientholds no per-miner or per-instance request state — only the pooledrequests.Session(utils/mirror/client.py). Sharing one instance across miners changes nothing about request content, retries, rate-limit handling, or scoring; it only reuses the connection pool instead of recreating it ~N times per round.Related Issues
None — this is a latent inefficiency found by inspection, not a tracked issue.
Type of Change
Testing
Added two tests in
tests/validator/oss_contributions/mirror/test_routing.pypinning the client-ownership contract:test_injected_client_is_reused_not_reconstructed— an injected client is threaded intoload_miner_prs/score_miner_prs, no newMirrorClientis constructed, and it is not closed by the callee.test_standalone_call_owns_and_closes_its_client— with no injected client, exactly one is constructed and closed (no leak).Full suite:
958 passed.ruff check,ruff format --check, andpyrightall clean.No CLI output is affected by this change.
Checklist