Implement remaining grain directory metrics#10124
Merged
Merged
Conversation
Reconnect grain directory cache lookup counters to the current lookup paths, restore LRU directory cache size observation, and remove the obsolete LocalLookup path which held stale metric call sites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the grain directory cache validation sent instrument since the current runtime no longer has an adaptive cache validation sender. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use immutable copy-on-write updates for cache size observers so observation can iterate without locks or per-observation array allocations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores and corrects grain directory-related metrics wiring in Orleans.Runtime by reconnecting counters/gauges to the current lookup/cache code paths, removing stale metrics/APIs, and adding coverage to ensure local synchronous lookup behavior works as intended.
Changes:
- Replace the removed
LocalLookupAPI withTryLocalLookupand wire local/cache/directory lookup counters into the active paths. - Restore LRU directory cache-size observation by registering an observable callback per cache instance and disposing registrations when caches are disposed.
- Remove the stale “cache validation sent” metric and count validation requests received on the distributed remote-directory compatibility path.
Show a summary per file
| File | Description |
|---|---|
| test/Orleans.Core.Tests/Directory/MockLocalGrainDirectory.cs | Updates test mock to implement the new TryLocalLookup API. |
| test/Orleans.Core.Tests/Directory/CachedGrainLocatorTests.cs | Adds a test ensuring TryLocalLookup can resolve authoritative local-partition entries on cache miss. |
| src/Orleans.Runtime/GrainDirectory/LruGrainDirectoryCache.cs | Registers cache-size observation and disposes the registration via IAsyncDisposable. |
| src/Orleans.Runtime/GrainDirectory/LocalGrainDirectory.cs | Introduces TryLocalLookup, wires lookup/cache metric increments, and removes the unused LocalLookup path/log messages. |
| src/Orleans.Runtime/GrainDirectory/ILocalGrainDirectory.cs | Removes LocalLookup and adds TryLocalLookup contract with updated documentation. |
| src/Orleans.Runtime/GrainDirectory/DistributedRemoteGrainDirectory.cs | Counts validation requests received for the compatibility LookUpMany path. |
| src/Orleans.Runtime/GrainDirectory/DhtGrainLocator.cs | Switches the synchronous fast-path to use TryLocalLookup (cache or authoritative local partition). |
| src/Orleans.Runtime/GrainDirectory/CachedGrainLocator.cs | Wires cache lookup issued/success counters into the locator cache path. |
| src/Orleans.Core/GrainDirectory/IGrainLocator.cs | Updates docstring to match “no async work” semantics (cache and/or authoritative local partition). |
| src/Orleans.Core/Diagnostics/Metrics/InstrumentNames.cs | Removes the unused DIRECTORY_VALIDATIONS_CACHE_SENT instrument name. |
| src/Orleans.Core/Diagnostics/Metrics/DirectoryInstruments.cs | Implements aggregated cache-size observation with multi-registration support and disposable registrations. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 1
Comment on lines
+88
to
+94
| var result = 0; | ||
| foreach (var observer in CacheSizeObservers) | ||
| { | ||
| result += observer.Observe(); | ||
| } | ||
|
|
||
| return result; |
ReubenBond
force-pushed
the
grain-directory-metrics
branch
from
268d984 to
d0bd422
Compare
ReubenBond
changed the title
ReubenBond
changed the title
This was referenced Jun 14, 2026
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.
Problem
Some grain directory metrics were defined but unreliable because their call sites were orphaned after locator/cache refactors or cache-size observation was no longer registered. The cache validation sent metric was also still defined even though the current runtime no longer has an adaptive cache-validation sender.
Solution
This wires cache/local lookup counters into the current cache lookup paths, restores cache-size observation for LRU directory caches, removes the unused LocalLookup path, removes the stale cache validation sent metric, and counts validation requests received by the distributed remote-directory compatibility path.
Review focus
Please focus on the metric semantics and whether the cache-size gauge aggregation is appropriate for multi-silo test hosts.