[OPIK-7256] fix(backend): make project size-estimate cache actually engage#7371
Open
thiagohora wants to merge 4 commits into
Open
[OPIK-7256] fix(backend): make project size-estimate cache actually engage#7371thiagohora wants to merge 4 commits into
thiagohora wants to merge 4 commits into
Conversation
…ngage The @Cacheable on getProjectMetadata never fired: Guice method interception cannot intercept private methods, and the cache name project_metadata was absent from cacheManager.caches (only an orphaned workspace_metadata entry existed), so even an intercepted call would have fallen back to the 1-second default TTL. The size-estimate query therefore ran on essentially every traces/spans find request. - make the method package-private so interception applies - rename the orphaned workspace_metadata config entry to project_metadata (env CACHE_MANAGER_PROJECT_METADATA_DURATION, PT1H) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
⏱️ pre-commit per-hook timing
⏭️ 39 skipped (no matching files changed)
|
Counting-stub DAO behind the real service in the app harness: a second call with the same key must not reach the DAO. Fails if the method becomes non-interceptable again (e.g. private) or the cache name is dropped from configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cache entry The interceptor drops the putAsync completion signal, so poll CacheManager.contains with a bounded timeout instead of sleeping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion All other @Cacheable methods are public; the class stays package-private so this widens nothing, and the comment keeps the not-private constraint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Details
The per-project size estimate (
WorkspaceMetadataDAO.getProjectMetadata— a 1000-span size sample plusuniq(id)over the whole project) is meant to be cached, but the cache never engaged, so the query ran on essentially every traces/spans find/search request and became one of the top read-volume queries on the ClickHouse cluster (~65 executions/min). Two stacked defects:@Cacheable(name = "project_metadata", ...)sits on a private method, and Guice method interception (which implements@Cacheable) cannot intercept private methods — the annotation was silently dead. Made the method package-private with a comment explaining why it must not be private.project_metadatawas absent fromcacheManager.caches; the config only carried an orphanedworkspace_metadataentry that no code references (stale name), so even an intercepted call would have used the 1-seconddefaultDuration. Renamed the entry toproject_metadata(envCACHE_MANAGER_PROJECT_METADATA_DURATION, default PT1H, matching the workingexperiment_metadatasibling).With the cache engaged, per-(workspace, project) executions drop from once per request to once per TTL window.
Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
WorkspaceMetadataServiceCacheTest: boots the app harness with a counting-stubWorkspaceMetadataDAOand asserts a repeatedgetProjectMetadatacall for the same key is served from the cache (DAO hit exactly once), and a different project misses. The test fails if the method becomes non-interceptable again (e.g. private) or if theproject_metadatacache name is dropped from configuration.mvn compileandmvn test-compilepass;mvn spotless:applyproduced no further changesexperiment_metadatacache — public method + configured name — demonstrates the working pattern this change aligns withsystem.query_log(7 days): ~657K executions/week of this query with per-request cardinality, confirming the cache was not engaging🤖 Generated with Claude Code
Documentation
No user-facing documentation impact. The new env var
CACHE_MANAGER_PROJECT_METADATA_DURATION(replacing the orphanedCACHE_MANAGER_WORKSPACE_METADATA_DURATION) is documented inline inconfig.yml.Note on the sample query itself: we evaluated also reworking the sample CTE's
ORDER BY (sorting key) DESC LIMIT 1000(it reads the whole project because reverse-order read never engages for this shape), but dropping the ORDER BY would pin the sample to the oldest ~1000 spans instead of the latest, so the estimate would stop tracking current span sizes. Keeping the query as-is; with the cache engaged it runs at most once per TTL per project.