Make cache_size gauge additive across caches sharing a name by stasimus · Pull Request #385 · evolution-gaming/scache

stasimus · 2026-09-02T22:18:21Z

Every cache reporting under the same name used to overwrite the cache_size gauge, so it showed whichever reported last. Each cache now adds its delta and withdraws its share on release. cache_size will show the true sum for a name, so dashboards will step up after upgrading.

Summary by CodeRabbit

New Features
- Added a resource-managed metrics factory for tracking cache sizes across multiple cache instances with the same metric name.
- Cache size contributions are automatically withdrawn when individual caches or metrics resources are released.
Bug Fixes
- Cache size metrics now reset to zero after cache release, including operations attempted on released caches.
Tests
- Added coverage for size aggregation, scheduled reporting, release behavior, and cache operations after release.

coderabbitai · 2026-09-02T22:19:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 858d09da-3721-4a0b-bc9a-b00f1f644bc6

📥 Commits

Reviewing files that changed from the base of the PR and between b72207d and 30952a1.

📒 Files selected for processing (2)

scache/src/main/scala/com/evolution/scache/CacheMetrics.scala
scache/src/test/scala/com/evolution/scache/CacheMetricsSpec.scala

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

Cache size metrics now aggregate contributions from multiple cache instances. Each cache tracks its previous reported size and withdraws its contribution when finalized. Tests cover aggregation, scheduled reporting, release behavior, and released-cache operations.

Changes

Cache size metrics

Layer / File(s)	Summary
Aggregate cache size reports `scache/src/main/scala/com/evolution/scache/CacheMetrics.scala`, `scache/src/test/scala/com/evolution/scache/CacheMetricsSpec.scala`	`CacheMetrics` tracks each cache’s previous size with an `AtomicInteger` and applies only size deltas to shared gauges. Tests verify aggregation across metric instances.
Reset size on cache release `scache/src/main/scala/com/evolution/scache/CacheMetered.scala`, `scache/src/test/scala/com/evolution/scache/CacheMeteredSpec.scala`, `scache/src/test/scala/com/evolution/scache/CacheSpec.scala`	`CacheMetered` reports size zero during resource finalization. Tests verify scheduled size reporting and zero-size reports after release operations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 30952

Cache-size gauges now aggregate contributions from same-named caches and withdraw released caches’ contributions. The covered aggregation and release behavior presents no remaining merge-readiness risk.

Sequence Diagram(s)

sequenceDiagram
  participant CacheMetered
  participant CacheMetrics
  participant sizeGauge
  CacheMetered->>CacheMetrics: Report cache size
  CacheMetrics->>sizeGauge: Apply per-cache size delta
  CacheMetered->>CacheMetrics: Report size 0 on finalization
  CacheMetrics->>sizeGauge: Withdraw cache contribution

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: making the cache_size gauge additive across caches that share a name.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-cache-size-gauge

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

stasimus · 2026-09-02T22:22:52Z

Some background on why this is needed.

A service can build several caches under one metrics name (say one per topic, then one per partition of that topic), all wrapped in withMetrics(cacheMetrics(name)). Each CacheMetered calls gauge.set(size) once a minute, so cache_size{name} was whichever instance reported last, sometimes a 2-entry cache, sometimes one with thousands. Noticed while comparing retention before/after the MapRef build.

The fix: each CacheMetrics from CacheMetrics.of remembers its last reported size and does gauge.inc(size - last), so the gauge is the sum over live instances. CacheMetered adds onFinalize(metrics.size(0)) before the Schedule, so it runs after the fiber is cancelled and a released cache withdraws its share.

Monad kept as the constraint, so still binary compatible with 6.0.2.

mr-git

I wonder, how often we have several caches reporting against the same set of metrics 🤔 ...

mr-git · 2026-09-03T11:13:45Z

+            ().pure[F].flatMap { _ =>
+              val delta = size - sizeReported.getAndSet(size)
+              sizeGauge1.inc(delta.toDouble).whenA(delta != 0)
+            }


This looks convoluted, feels that usage of Ref might look better than direct usage of AtomicInteger.

Is it done this way in attempt to NOT change the public API? If so, with #369 we will have to change the public API - we could change this one too (from : Monad to : Async?)

As an alternative, we could introduce the def make[F[_]: Concurrent] which builds the Ref-based and deprecate the "expanded" of like:

@deprecated(message = "use `make`", since = "2026-09-03") def of[F[_]: Concurrent]( collectorRegistry: CollectorRegistry[F], prefix: Prefix = Prefix.Default, sizeReported: Option[Ref[F, Int]] = None, ): Resource[F, Name => CacheMetrics[F]]

In .of we can keep the old behaviour on None, but on next major bump we can drop the .of.

Not sure, if it is worth the effort, but usage of Ref feels to be nicer fit that relying on raw AtomicInteger

one more option - just introduce the def make as an alternative, with copy-paste-improved code from deprecated .of.

Yes, the AtomicInteger was there to keep of on Monad and the return type intact, since this PR is meant to ship in 6.x.

Went with the third option, 30952a1: make[F: Concurrent] returns Name => Resource[F, CacheMetrics[F]], keeps the reported size in a Ref and withdraws it in its own finalizer, so callers get the right numbers without remembering to report 0. of is deprecated but keeps the additive fix, so existing users get correct sums without a code change. Both share the collector wiring, no copy paste. On the 7.0.0 branch of can go.

It seems to be OK, but very convoluted. If we plan to discard .of right after release of 6.1.0, then I'd go with copy-paste-amend implementation with less wrappers and indirections, which will be harder to roll-back after 6.1.0

mr-git · 2026-09-03T11:16:12Z

        a <- cache.get(0)
        _ <- IO { a shouldEqual none[Int] }
-        _ <- metrics.expect(metrics.expectedGet(hit = false) -> 1)
+        _ <- metrics.expect(metrics.expectedGet(hit = false) -> 1, metrics.expectedSize(0) -> 1)


IMHO, here and in whole file, it would look better, if in case of 2+ "expects", we'd go multi-line, though I'd prefer to go multi-line in all cases, even with single check, like:

_ <- metrics.expect( // all checks listed here, one per line )

mr-git

LGTM, though I do not like the increased complexity of code. I'd go with deprecation of .of and improved duplicate of .make in 6.1.0 and clean deletion of .of in 7.0.0

Make cache_size gauge additive across caches sharing a name

dc4a637

Use whenA for conditional gauge update

b72207d

mr-git reviewed Sep 3, 2026

View reviewed changes

Add Ref-based CacheMetrics.make, deprecate of

30952a1

mr-git approved these changes Sep 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

Make cache_size gauge additive across caches sharing a name - #385

Make cache_size gauge additive across caches sharing a name#385
stasimus wants to merge 3 commits into
masterfrom
fix-cache-size-gauge

stasimus commented Sep 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 2, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

stasimus commented Sep 2, 2026 •

edited

Loading

Uh oh!

mr-git left a comment

Uh oh!

mr-git Sep 3, 2026

Uh oh!

mr-git Sep 3, 2026

Uh oh!

mr-git Sep 3, 2026

Uh oh!

stasimus Sep 3, 2026

Uh oh!

mr-git Sep 3, 2026

Uh oh!

mr-git Sep 3, 2026

Uh oh!

mr-git left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

stasimus commented Sep 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

stasimus commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mr-git left a comment

Choose a reason for hiding this comment

Uh oh!

mr-git Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mr-git Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mr-git Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

stasimus Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mr-git Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mr-git Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

mr-git left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

stasimus commented Sep 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 2, 2026 •

edited

Loading

stasimus commented Sep 2, 2026 •

edited

Loading