Skip to content

fix: prevent regexp cache charsCurrent inflation on duplicate Put#68

Merged
f41gh7 merged 2 commits intomasterfrom
fix/regexp-cache-duplicate-put-accounting
Apr 15, 2026
Merged

fix: prevent regexp cache charsCurrent inflation on duplicate Put#68
f41gh7 merged 2 commits intomasterfrom
fix/regexp-cache-duplicate-put-accounting

Conversation

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot commented Apr 13, 2026

Summary

  • Fix regexpCache.Put to only increment charsCurrent when the key is new, preventing size counter inflation from concurrent duplicate inserts
  • Add test cases covering duplicate-key Put to verify correct accounting

Problem

CompileRegexp performs a non-atomic Get/compile/Put sequence. When multiple goroutines miss on Get for the same regexp concurrently, each calls Put, which blindly incremented charsCurrent without checking if the key already existed. This inflated the size counter while len(rc.m) stayed at 1, causing premature cache eviction and unnecessary regexp recompiles.

Fix

Added an existence check before incrementing charsCurrent:

if _, exists := rc.m[regexp]; !exists {
    rc.charsCurrent += len(regexp)
}
rc.m[regexp] = rcv

Test plan

  • Extended TestRegexpCache with duplicate-key inputs
  • Added TestRegexpCacheDuplicatePut verifying triple-Put only counts chars once
  • All tests pass with -race detector
  • go vet clean

🤖 Generated with Claude Code


Summary by cubic

Fix regexpCache.Put to count chars only for new keys and add a fast path for existing entries. Prevents counter inflation under concurrency and premature evictions; tests cover duplicates.

Written for commit 958928e. Summary will update on new commits.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.81%. Comparing base (8ba28d7) to head (958928e).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage   89.67%   90.81%   +1.14%     
==========================================
  Files          11       11              
  Lines        2984     3257     +273     
==========================================
+ Hits         2676     2958     +282     
+ Misses        210      204       -6     
+ Partials       98       95       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor Author

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

@f41gh7 f41gh7 self-assigned this Apr 14, 2026
@f41gh7 f41gh7 self-requested a review April 14, 2026 19:56
* remove excessive test
* add fast path

Signed-off-by: f41gh7 <nik@victoriametrics.com>
@f41gh7
Copy link
Copy Markdown
Contributor

f41gh7 commented Apr 15, 2026

CompileRegexp performs a non-atomic Get/compile/Put sequence. When multiple goroutines miss on Get for the same regexp concurrently, each calls Put, which blindly incremented charsCurrent without checking if the key already existed. This inflated the size counter while len(rc.m) stayed at 1, causing premature cache eviction and unnecessary regexp recompiles.

This commit adds a check, if parsed regex was already added by concurrent goroutine.

@f41gh7 f41gh7 merged commit b60c155 into master Apr 15, 2026
8 checks passed
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