Skip to content

fix(cache): make in memory cache global#4560

Open
MasterPtato wants to merge 1 commit intomainfrom
04-05-fix_cache_make_in_memory_cache_global
Open

fix(cache): make in memory cache global#4560
MasterPtato wants to merge 1 commit intomainfrom
04-05-fix_cache_make_in_memory_cache_global

Conversation

@MasterPtato
Copy link
Copy Markdown
Contributor

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@MasterPtato MasterPtato requested a review from NathanFlurry April 5, 2026 19:24
@railway-app
Copy link
Copy Markdown

railway-app bot commented Apr 5, 2026

🚅 Deployed to the rivet-pr-4560 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web Apr 7, 2026 at 1:31 am
mcp-hub ✅ Success (View Logs) Web Apr 5, 2026 at 7:25 pm
website ❌ Build Failed (View Logs) Web Apr 5, 2026 at 7:25 pm
frontend-cloud ❌ Build Failed (View Logs) Web Apr 5, 2026 at 7:24 pm
ladle ❌ Build Failed (View Logs) Web Apr 5, 2026 at 7:24 pm
frontend-inspector ❌ Build Failed (View Logs) Web Apr 5, 2026 at 7:24 pm

@claude
Copy link
Copy Markdown

claude bot commented Apr 5, 2026

PR Review: fix(cache): make in memory cache global. Summary: The PR makes the in-memory cache and in-flight map global singletons using OnceLock. The tracing improvements are clean. Issues: (1) Silent capacity mismatch in InMemoryDriver::new (driver.rs:147) - After the first call to InMemoryDriver::new(max_capacity), the max_capacity argument is silently ignored by subsequent calls since OnceLock::get_or_init is a no-op after initialization. Consider removing max_capacity entirely or asserting it matches on subsequent calls. (2) Test isolation risk - Both CACHE and IN_FLIGHT are process-level statics. Tests creating multiple InMemoryDriver or CacheInner instances share the same underlying state, so state from one test can leak into another. (3) Shared IN_FLIGHT across CacheInner instances (inner.rs:51) - If multiple CacheInner instances are created for different subsystems, they share the same in-flight map. A comment documenting this assumption would help future readers. Positives: Empty check before cache.driver.get() avoids a no-op round-trip; tracing improvements are cleaner; eviction listener is useful for observability. Minor: InMemoryDriver is now an empty struct and could be a ZST.

@MasterPtato MasterPtato force-pushed the 04-05-fix_cache_make_in_memory_cache_global branch from 9f06a14 to e815bd1 Compare April 6, 2026 19:54
@NathanFlurry NathanFlurry force-pushed the 04-05-fix_cache_make_in_memory_cache_global branch from e815bd1 to 9f06a14 Compare April 6, 2026 20:21
@MasterPtato MasterPtato force-pushed the 04-05-fix_cache_make_in_memory_cache_global branch from 9f06a14 to b15930f Compare April 6, 2026 22:18
@MasterPtato MasterPtato mentioned this pull request Apr 6, 2026
11 tasks
@NathanFlurry NathanFlurry force-pushed the 04-05-fix_cache_make_in_memory_cache_global branch from b15930f to 9f06a14 Compare April 6, 2026 23:59
@MasterPtato MasterPtato force-pushed the 04-05-fix_cache_make_in_memory_cache_global branch from 9f06a14 to b15930f Compare April 7, 2026 00:04
@NathanFlurry NathanFlurry mentioned this pull request Apr 7, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant