[HF 9.3.13] CLDSRV-979: rate limit pinned request logger

lib/api/apiUtils/rateLimit/refillJob.js

@@ -3,14 +3,10 @@
 let refillTimer = null;
 
 // Refill interval in milliseconds (how often to check and refill buckets)
-const REFILL_INTERVAL_MS = process.env.REFILL_INTERVAL_MS
-    ? parseInt(process.env.REFILL_INTERVAL_MS, 10)
-    : 100;
+const REFILL_INTERVAL_MS = process.env.REFILL_INTERVAL_MS ? parseInt(process.env.REFILL_INTERVAL_MS, 10) : 100;
 
 // Cleanup interval for expired buckets (every 10 seconds)
-const CLEANUP_INTERVAL_MS = process.env.CLEANUP_INTERVAL_MS
-    ? parseInt(process.env.CLEANUP_INTERVAL_MS, 10)
-    : 10000;
+const CLEANUP_INTERVAL_MS = process.env.CLEANUP_INTERVAL_MS ? parseInt(process.env.CLEANUP_INTERVAL_MS, 10) : 10000;
 
 let cleanupCounter = 0;
 
@@ -38,19 +34,22 @@
         checked++;
 
         // Trigger async refill if needed (non-blocking)
-        const promise = bucket.refillIfNeeded().then(bucketRefilled => {
-            // Check if refill actually happened
-            if (bucketRefilled) {
-                refilled++;
-            }
-        }).catch(err => {
-            logger.error('error refilling token bucket', {
-                bucketName,
-                method: 'rateLimit.refillTokenBuckets',
-                error: err.message,
-                stack: err.stack,
+        const promise = bucket
+            .refillIfNeeded(logger)
+            .then(bucketRefilled => {
+                // Check if refill actually happened
+                if (bucketRefilled) {
+                    refilled++;
+                }
+            })
+            .catch(err => {
+                logger.error('error refilling token bucket', {
+                    bucketName,
+                    method: 'rateLimit.refillTokenBuckets',
+                    error: err.message,
+                    stack: err.stack,
+                });
             });
-        });
 
         refillPromises.push(promise);
     }

lib/api/apiUtils/rateLimit/tokenBucket.js

@@ -12,7 +12,7 @@
 
 const util = require('util');
 
-const { instance: redisClient } = require('./client');
+const rateLimitClient = require('./client');
 const { config } = require('../../../Config');
 const { calculateInterval } = require('./gcra');
 
@@ -23,12 +23,11 @@ const tokenBuckets = new Map();
  * Per-resourceClass+resourceID+measure token bucket for a single worker
  */
 class WorkerTokenBucket {
-    constructor(resourceClass, resourceId, measure, limitConfig, log) {
+    constructor(resourceClass, resourceId, measure, limitConfig) {
         this.resourceClass = resourceClass;
         this.resourceId = resourceId;
         this.measure = measure;
         this.limitConfig = limitConfig;
-        this.log = log;
 
         this.bufferSize = config.rateLimiting?.tokenBucketBufferSize; // Max tokens to hold
         this.refillThreshold = config.rateLimiting?.tokenBucketRefillThreshold; // Trigger refill when below this
@@ -43,7 +42,8 @@ class WorkerTokenBucket {
     }
 
     updateLimit(updatedConfig) {
-        if (this.limitConfig.limit !== updatedConfig.limit ||
+        if (
+            this.limitConfig.limit !== updatedConfig.limit ||
             this.limitConfig.burstCapacity !== updatedConfig.burstCapacity
         ) {
             const oldConfig = this.limitConfig;
@@ -73,9 +73,13 @@ class WorkerTokenBucket {
      * Check if refill is needed and trigger async refill
      * Called by background job every 100ms
      *
+     * The logger is deliberately taken per call and never stored on the
+     * bucket: a retained request logger buffers entries forever.
+     *
+     * @param {object} log - Logger instance, supplied by the caller
      * @returns {Promise<boolean>}
      */
-    async refillIfNeeded() {
+    async refillIfNeeded(log) {
         // Already refilling, skip
         if (this.refillInProgress) {
             return false;
@@ -99,6 +103,9 @@ class WorkerTokenBucket {
 
             // Calculate GCRA parameters
             let granted = requested;
+            // Read the instance at call time rather than destructuring it at
+            // module load, so tests can substitute a fake client.
+            const redisClient = rateLimitClient.instance;
             if (redisClient.isReady()) {
                 // Request tokens from Redis (atomic GCRA enforcement)
                 granted = await util.promisify(redisClient.grantTokens.bind(redisClient))(
@@ -113,14 +120,11 @@ class WorkerTokenBucket {
                 // Connection to redis has failed in some way.
                 // Client will be reconnecting in the background.
                 // We grant the requested amount of tokens anyway to avoid degrading service availability.
-                this.log.warn(
-                    'rate limit redis client not connected. granting tokens anyway to avoid service degradation',
-                    {
-                        resourceClass: this.resourceClass,
-                        resourceId: this.resourceId,
-                        measure: this.measure,
-                    },
-                );
+                log.warn('rate limit redis client not connected. granting tokens anyway to avoid service degradation', {
+                    resourceClass: this.resourceClass,
+                    resourceId: this.resourceId,
+                    measure: this.measure,
+                });
             }
 
             // Add granted tokens to buffer
@@ -129,7 +133,7 @@ class WorkerTokenBucket {
             this.lastRefillTime = Date.now();
             const duration = this.lastRefillTime - startTime;
 
-            this.log.debug('Token refill completed', {
+            log.debug('Token refill completed', {
                 resourceClass: this.resourceClass,
                 resourceId: this.resourceId,
                 measure: this.measure,
@@ -141,7 +145,7 @@ class WorkerTokenBucket {
 
             // Warn if refill took too long or granted too few
             if (duration > 100) {
-                this.log.warn('Slow token refill detected', {
+                log.warn('Slow token refill detected', {
                     resourceClass: this.resourceClass,
                     resourceId: this.resourceId,
                     measure: this.measure,
@@ -150,7 +154,7 @@ class WorkerTokenBucket {
             }
 
             if (granted === 0 && requested > 0) {
-                this.log.trace('Token refill denied - quota exhausted', {
+                log.trace('Token refill denied - quota exhausted', {
                     resourceClass: this.resourceClass,
                     resourceId: this.resourceId,
                     measure: this.measure,
@@ -162,7 +166,7 @@ class WorkerTokenBucket {
 
             return true;
         } catch (err) {
-            this.log.error('Token refill failed', {
+            log.error('Token refill failed', {
                 resourceClass: this.resourceClass,
                 resourceId: this.resourceId,
                 measure: this.measure,
@@ -190,7 +194,7 @@ function getTokenBucket(resourceClass, resourceId, measure, limitConfig, log) {
     const cacheKey = `${resourceClass}:${resourceId}:${measure}`;
     let bucket = tokenBuckets.get(cacheKey);
     if (!bucket) {
-        bucket = new WorkerTokenBucket(resourceClass, resourceId, measure, limitConfig, log);
+        bucket = new WorkerTokenBucket(resourceClass, resourceId, measure, limitConfig);
         tokenBuckets.set(cacheKey, bucket);
 
         log.debug('Created token bucket', {
@@ -225,7 +229,7 @@ function getAllTokenBuckets() {
  * Clean up expired token buckets
  * Called periodically by cleanup job
  *
- * @param {number} maxIdleMs - Remove buckets idle for more than this duration
+ * @param {number} maxIdleMs - Remove buckets unused by any request for more than this duration
  * @returns {number} Number of buckets removed
  */
 function cleanupTokenBuckets(maxIdleMs = 60000) {
@@ -234,7 +238,7 @@ function cleanupTokenBuckets(maxIdleMs = 60000) {
 
     for (const [key, bucket] of tokenBuckets.entries()) {
         const idleTime = now - bucket.lastRefillTime;
-        if (idleTime > maxIdleMs && bucket.tokens === 0) {
+        if (idleTime > maxIdleMs) {
             toRemove.push(key);
         }
     }

package.json

@@ -1,6 +1,6 @@
 {
     "name": "@zenko/cloudserver",
-    "version": "9.3.13",
+    "version": "9.3.13-1",
     "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
     "main": "index.js",
     "engines": {