Skip to content

invalidate local cache on is_negative_hits - #1230

Open
oclaw wants to merge 1 commit into
envoyproxy:mainfrom
oclaw:negative-hits-flag-invalidation
Open

invalidate local cache on is_negative_hits#1230
oclaw wants to merge 1 commit into
envoyproxy:mainfrom
oclaw:negative-hits-flag-invalidation

Conversation

@oclaw

@oclaw oclaw commented Sep 1, 2026

Copy link
Copy Markdown

Problem

is_negative_hits (#1140) lets a descriptor decrement previously consumed quota, but refunds are invisible to the local over-limit cache: once a replica cached a key as over limit, a refund that freed quota in Redis kept being answered OVER_LIMIT from every replica's local cache until the window ended which degrades the feature on long windows

Suggested design

  • Introduce feature flag ENABLE_NEGATIVE_HITS (default off). While off, a request containing a negative-hit descriptor is rejected with UNIMPLEMENTED
  • Refuse to boot the one unfixable combination: negative hits + memcached + local cache (this PR does not add any support for the invalidations with memcached as a backend). Memcached refunds without a local cache enabled keep working.
  • Add Redis pub-sub backed invalidation routine: at-most-once guarantee, single added connection per replica + local cache gen-based guard for eliminating races between invalidation and stale insert

P.S. Agent also added some missing tests for touched components, look fine

// DecrementScript atomically decrements a rate limit counter, floored at 0.
// If the key does not exist there is nothing to refund, so it returns 0 without
// creating a phantom key.
const DecrementScript = `

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is not really a common limiter part, moved it closer to Redis backend impl

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Reused driver.go as much as possible but this routine has quite independent connection cycle so decided not to bother too much

if hitsAddends[i].IsNegative {
pipelineAppendDecrement(client, p, cacheKey.Key, hitsAddends[i].Value, &results[i], expirationSeconds)
// The subscriber listens only on the main Redis.
publishInvalidation := this.publishInvalidations && !onPerSecondRedis

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

dedicated per second redis assume high load and redis-backed refunds are barely effective here, not doing them (and the invalidator only listens main redis at the moment)

@oclaw
oclaw force-pushed the negative-hits-flag-invalidation branch 2 times, most recently from 1e9ab1b to 0602802 Compare September 1, 2026 14:45
…cal cache on refunds

Signed-off-by: Daniil Kogtev <oclaw@icloud.com>
@oclaw
oclaw force-pushed the negative-hits-flag-invalidation branch from 0602802 to ef92df1 Compare September 1, 2026 14:51
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