From 2cf6db99ceb0b8321f29f4a4cde18dbb4baec1e0 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Fri, 7 Aug 2026 09:47:09 +0100 Subject: [PATCH] Fix timing-fragile Valkey rate-limit boundary test (#7302) # Description of Changes Fix timing-fragile Valkey rate-limit boundary test --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --- .../valkey/LiveValkeyIntegrationTest.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/cluster/valkey/LiveValkeyIntegrationTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/cluster/valkey/LiveValkeyIntegrationTest.java index c26528a715..0e6cb2f4de 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/cluster/valkey/LiveValkeyIntegrationTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/cluster/valkey/LiveValkeyIntegrationTest.java @@ -298,24 +298,44 @@ class LiveValkeyIntegrationTest { ValkeyRateLimitStore store = newRateLimitStore(factoryA); String key = "boundary-" + java.util.UUID.randomUUID(); long capacity = 5; - Duration window = Duration.ofMillis(500); + // refillGreedy tops the bucket up continuously, one token every window/capacity. A 500ms + // window left the drain loop only 100ms before a 6th token appeared, so a slow Valkey + // round-trip broke the count; 4s spaces refills 800ms apart, clear of any burst. + Duration window = Duration.ofSeconds(4); + long refillIntervalMs = window.toMillis() / capacity; + long drainStart = System.nanoTime(); int firstAllowed = 0; for (int i = 0; i < 10; i++) { if (store.tryConsume(key, capacity, window).allowed()) firstAllowed++; } - assertEquals(capacity, firstAllowed, "must allow exactly capacity tokens initially"); + long drainMs = (System.nanoTime() - drainStart) / 1_000_000; + // Refill never pauses, so a slow drain earns extra tokens honestly - allow exactly the + // number the elapsed time can have produced and no more. + long earned = drainMs / refillIntervalMs; + assertTrue( + firstAllowed >= capacity && firstAllowed <= capacity + earned, + "initial burst must be capacity (" + + capacity + + ") plus at most the " + + earned + + " token(s) refilled during a " + + drainMs + + "ms drain, got " + + firstAllowed); - Thread.sleep(window.toMillis() + 50); + // A fixed-window limiter would hand back a whole fresh capacity at the boundary; a token + // bucket hands back one token per refill interval. + Thread.sleep(refillIntervalMs + 200); int secondAllowed = 0; - long start = System.nanoTime(); - for (int i = 0; i < 20 && (System.nanoTime() - start) < 20_000_000L; i++) { + for (int i = 0; i < 10; i++) { if (store.tryConsume(key, capacity, window).allowed()) secondAllowed++; } assertTrue( - secondAllowed <= capacity, - "token-bucket must not let a fresh full capacity be consumed instantly across" - + " the boundary; got " + secondAllowed >= 1 && secondAllowed < capacity, + "one refill interval must yield about one token, not a fresh full window of " + + capacity + + "; got " + secondAllowed); }