Skip to content

impr(CLDSRV-852): Refactor WorkerTokenBucket#6108

Merged
tmacro merged 3 commits intoimprovement/CLDSRV-869/account_limitingfrom
improvement/CLDSRV-852/refactor_token_bucket_2
Mar 26, 2026
Merged

impr(CLDSRV-852): Refactor WorkerTokenBucket#6108
tmacro merged 3 commits intoimprovement/CLDSRV-869/account_limitingfrom
improvement/CLDSRV-852/refactor_token_bucket_2

Conversation

@tmacro
Copy link
Copy Markdown
Contributor

@tmacro tmacro commented Mar 13, 2026 •
edited
Loading

  • Update RateLimitClient.grantToken() params
  • Cleanup calculateInterval helper
  • Refactor WorkerTokenBucket
    • Change constructor to accept resourceClass, resourceId, measure
    • Precalculate interval for reuse in constructor
    • Add hasCapacity() method
    • Add updateLimit() method to encasulate limit change logic
    • Remove unused mechanism to record request durations
    • Simplify refill logic

Note for revewers: S3 API unit tests are still expected to fail

claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026 •
edited
Loading

❌ 109 Tests Failed:

Tests completed Failed Passed Skipped
2695 109 2586 0
View the top 3 failed test(s) by shortest run time 
should abort a multipart upload on real AWS location withbucketMatch set to false::Multipart Upload API with AWS Backend should abort a multipart upload on real AWS location withbucketMatch set to false
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should abort a multipart upload on real AWS::Multipart Upload API with AWS Backend should abort a multipart upload on real AWS
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should complete MPU on file with same key as object put to AWS::Multipart Upload API with AWS Backend should complete MPU on file with same key as object put to AWS
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should complete a multipart upload on real AWS location with bucketMatch set to false::Multipart Upload API with AWS Backend should complete a multipart upload on real AWS location with bucketMatch set to false
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should complete a multipart upload::Multipart Upload API with AWS Backend with mpu initiated on legacy version should complete a multipart upload
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should copy an object as a part to a multipart upload::Multipart Upload API with AWS Backend with mpu initiated on legacy version should copy an object as a part to a multipart upload
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to file based on mpu location::ObjectCopyPutPart API with multiple backends should copy part to file based on mpu location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to file based on request endpoint::ObjectCopyPutPart API with multiple backends should copy part to file based on request endpoint
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to mem based on bucket location::ObjectCopyPutPart API with multiple backends should copy part to mem based on bucket location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to mem based on mpu location::ObjectCopyPutPart API with multiple backends should copy part to mem based on mpu location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should initiate a multipart upload on AWS location with bucketMatch equals false::Multipart Upload API with AWS Backend should initiate a multipart upload on AWS location with bucketMatch equals false
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should initiate a multipart upload with tags on AWS::Multipart Upload API with AWS Backend should initiate a multipart upload with tags on AWS
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should list all multipart uploads on all backends::Multipart Upload API with AWS Backend should list all multipart uploads on all backends
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should list multipart upload parts::Multipart Upload API with AWS Backend with mpu initiated on legacy version should list multipart upload parts
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should list the parts of a multipart upload on real AWS location with bucketMatch set to false::Multipart Upload API with AWS Backend should list the parts of a multipart upload on real AWS location with bucketMatch set to false
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should list the parts of a multipart upload on real AWS::Multipart Upload API with AWS Backend should list the parts of a multipart upload on real AWS
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should only return number of parts equal to specified maxParts::Multipart Upload API with AWS Backend should only return number of parts equal to specified maxParts
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to AWS based on bucket location with bucketMatch set to false::objectPutPart API with multiple backends should put a part to AWS based on bucket location with bucketMatch set to false
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to AWS based on bucket location::objectPutPart API with multiple backends should put a part to AWS based on bucket location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to AWS based on mpu location::objectPutPart API with multiple backends should put a part to AWS based on mpu location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to mem based on bucket location::objectPutPart API with multiple backends should put a part to mem based on bucket location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to file based on bucket location::objectPutAPI with multiple backends should put an object to file based on bucket location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to file::objectPutAPI with multiple backends should put an object to file
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to mem based on bucket location::objectPutAPI with multiple backends should put an object to mem based on bucket location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to us-east-1 which is file based on bucket location if no locationConstraint provided::objectPutAPI with multiple backends should put an object to us-east-1 which is file based on bucket location if no locationConstraint provided
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should replace part if two parts uploaded with same part number to AWS::objectPutPart API with multiple backends should replace part if two parts uploaded with same part number to AWS
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should return ServiceUnavailable if MPU deleted directly from AWS and try to complete from S3::Multipart Upload API with AWS Backend should return ServiceUnavailable if MPU deleted directly from AWS and try to complete from S3
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should return error on abort of MPU that does not exist::Multipart Upload API with AWS Backend should return error on abort of MPU that does not exist
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should return invalidPart error::Multipart Upload API with AWS Backend should return invalidPart error
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should return invalidPartOrder error::Multipart Upload API with AWS Backend should return invalidPartOrder error
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should upload a part to file based on mpu location::objectPutPart API with multiple backends should upload a part to file based on mpu location
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
should upload part based on mpu location even if part location constraint is specified ::objectPutPart API with multiple backends should upload part based on mpu location even if part location constraint is specified
Stack Traces | 0s run time 
extractAndCacheRateLimitConfig is not a function
"after each" hook for "DELETE .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should delete the XML file"::Veeam routes: veeam DELETE routes: "after each" hook for "DELETE .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should delete the XML file"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"after each" hook for "GET .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the expected XML file"::Veeam routes: veeam GET routes: "after each" hook for "GET .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the expected XML file"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"after each" hook for "HEAD .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the existing XML file metadata"::Veeam routes: veeam HEAD routes: "after each" hook for "HEAD .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the existing XML file metadata"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"after each" hook for "should successfully replicate a version"::backbeat routes for replication (cross account) "after each" hook for "should successfully replicate a version"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before each" hook for "should successfully replicate a version"::backbeat routes for replication (cross account) "before each" hook for "should successfully replicate a version"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before each" hook for "should successfully replicate a version"::backbeat routes for replication (same account) "before each" hook for "should successfully replicate a version"
Stack Traces | 0.001s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should abort a multipart upload::Multipart Upload API with AWS Backend with mpu initiated on legacy version should abort a multipart upload
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should allow a restore if the quota is full but the objet fits with its reserved storage space::quota evaluation with scuba metrics should allow a restore if the quota is full but the objet fits with its reserved storage space
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should allow creating mpu with SSE header in encrypted bucket::KMIP backed server-side encryption should allow creating mpu with SSE header in encrypted bucket
Stack Traces | 0.001s run time 
connect ECONNREFUSED 127.0.0.1:8000
should allow object PUT with SSE header in encrypted bucket::KMIP backed server-side encryption should allow object PUT with SSE header in encrypted bucket
Stack Traces | 0.001s run time 
connect ECONNREFUSED 127.0.0.1:8000
should allow object copy with SSE header in encrypted bucket::KMIP backed server-side encryption should allow object copy with SSE header in encrypted bucket
Stack Traces | 0.001s run time 
connect ECONNREFUSED 127.0.0.1:8000
should allow writes after deleting data with quotas below the current number of inflights::quota evaluation with scuba metrics should allow writes after deleting data with quotas below the current number of inflights
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should allow writes after multi-deleting data with quotas below the current number of inflights::quota evaluation with scuba metrics should allow writes after multi-deleting data with quotas below the current number of inflights
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should be successful initiating MPU on AWS with Scality S3 versioning enabled::Multipart Upload API with AWS Backend should be successful initiating MPU on AWS with Scality S3 versioning enabled
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should complete MPU on AWS with same key as object put to file::Multipart Upload API with AWS Backend should complete MPU on AWS with same key as object put to file
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should complete a multipart upload on real AWS::Multipart Upload API with AWS Backend should complete a multipart upload on real AWS
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part an object on AWS location that has bucketMatch equals false to a mpu with a different AWS location::ObjectCopyPutPart API with multiple backends should copy part an object on AWS location that has bucketMatch equals false to a mpu with a different AWS location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part an object on AWS to a mpu with a different AWS location that has bucketMatch equals false::ObjectCopyPutPart API with multiple backends should copy part an object on AWS to a mpu with a different AWS location that has bucketMatch equals false
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to AWS based on bucket location::ObjectCopyPutPart API with multiple backends should copy part to AWS based on bucket location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to AWS based on mpu location::ObjectCopyPutPart API with multiple backends should copy part to AWS based on mpu location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to file based on bucket location::ObjectCopyPutPart API with multiple backends should copy part to file based on bucket location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should copy part to mem from AWS based on mpu location::ObjectCopyPutPart API with multiple backends should copy part to mem from AWS based on mpu location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should decrease the inflights when performing multi object delete::quota evaluation with scuba metrics should decrease the inflights when performing multi object delete
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should not increase the inflights when the object is being rewritten with a smaller object::quota evaluation with scuba metrics should not increase the inflights when the object is being rewritten with a smaller object
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should not update the inflights if the API errored after evaluating quotas (deletion)::quota evaluation with scuba metrics should not update the inflights if the API errored after evaluating quotas (deletion)
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should only list parts after PartNumberMarker::Multipart Upload API with AWS Backend should only list parts after PartNumberMarker
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to AWS based on bucket location with bucketMatch set to true::objectPutPart API with multiple backends should put a part to AWS based on bucket location with bucketMatch set to true
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to file based on bucket location::objectPutPart API with multiple backends should put a part to file based on bucket location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to file based on request endpoint::objectPutPart API with multiple backends should put a part to file based on request endpoint
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put a part to mem based on mpu location::objectPutPart API with multiple backends should put a part to mem based on mpu location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to AWS based on bucket location::objectPutAPI with multiple backends should put an object to AWS based on bucket location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to AWS::objectPutAPI with multiple backends should put an object to AWS
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to Azure based on bucket location::objectPutAPI with multiple backends should put an object to Azure based on bucket location
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to azure::objectPutAPI with multiple backends should put an object to azure
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to mem::objectPutAPI with multiple backends should put an object to mem
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should put an object to sproxyd::objectPutAPI with multiple backends should put an object to sproxyd
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should reduce inflights when aborting MPU::quota evaluation with scuba metrics should reduce inflights when aborting MPU
Stack Traces | 0.001s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return an error on listParts of deleted MPU::Multipart Upload API with AWS Backend should return an error on listParts of deleted MPU
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should return entityTooSmall error::Multipart Upload API with AWS Backend should return entityTooSmall error
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
should store a part even if the MPU was initiated on legacy version::objectPutPart API with multiple backends should store a part even if the MPU was initiated on legacy version
Stack Traces | 0.001s run time 
extractAndCacheRateLimitConfig is not a function
"after all" hook for "PUT .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml should fail if invalid credentials are sent"::Veeam routes: veeam PUT routes: "after all" hook for "PUT .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml should fail if invalid credentials are sent"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"after each" hook for "should successfully replicate a version"::backbeat routes for replication (same account) "after each" hook for "should successfully replicate a version"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before all" hook for "PUT .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml"::Veeam routes: veeam PUT routes: "before all" hook for "PUT .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before each" hook for "DELETE .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should delete the XML file"::Veeam routes: veeam DELETE routes: "before each" hook for "DELETE .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should delete the XML file"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before each" hook for "GET .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the expected XML file"::Veeam routes: veeam GET routes: "before each" hook for "GET .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the expected XML file"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"before each" hook for "HEAD .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the existing XML file metadata"::Veeam routes: veeam HEAD routes: "before each" hook for "HEAD .system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml should return the existing XML file metadata"
Stack Traces | 0.002s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
object metadata for newly stored object should have dataStoreName if copying to mem based on bucket location::ObjectCopy API with multiple backends object metadata for newly stored object should have dataStoreName if copying to mem based on bucket location
Stack Traces | 0.002s run time 
extractAndCacheRateLimitConfig is not a function
should allow writes after deleting data with quotas::quota evaluation with scuba metrics should allow writes after deleting data with quotas
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should create an encrypted bucket and upload an object::KMIP backed server-side encryption should create an encrypted bucket and upload an object
Stack Traces | 0.002s run time 
connect ECONNREFUSED 127.0.0.1:8000
should initiate a multipart upload on real AWS::Multipart Upload API with AWS Backend should initiate a multipart upload on real AWS
Stack Traces | 0.002s run time 
extractAndCacheRateLimitConfig is not a function
should not evaluate quotas if the backend is not available::quota evaluation with scuba metrics should not evaluate quotas if the backend is not available
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should not return QuotaExceeded if the quota is not exceeded::quota evaluation with scuba metrics should not return QuotaExceeded if the quota is not exceeded
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should only evaluate quota and not update inflights for PutObject with the x-scal-s3-version-id header::quota evaluation with scuba metrics should only evaluate quota and not update inflights for PutObject with the x-scal-s3-version-id header
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should reduce inflights when completing MPU with fewer parts than uploaded::quota evaluation with scuba metrics should reduce inflights when completing MPU with fewer parts than uploaded
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return QuotaExceeded when trying to CopyObject in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to CopyObject in a bucket with quota
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return QuotaExceeded when trying to complete MPU in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to complete MPU in a bucket with quota
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return QuotaExceeded when trying to copy a part in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to copy a part in a bucket with quota
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return QuotaExceeded when trying to copyObject in a versioned bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to copyObject in a versioned bucket with quota
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return QuotaExceeded when trying to restore an object in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to restore an object in a bucket with quota
Stack Traces | 0.002s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should not update the inflights if the quota check is passing but the object is already restored::quota evaluation with scuba metrics should not update the inflights if the quota check is passing but the object is already restored
Stack Traces | 0.003s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
"after all" hook in "backbeat routes"::backbeat routes "after all" hook in "backbeat routes"
Stack Traces | 0.004s run time 
Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should return MethodNotAllowed for invalid request::Veeam routes: veeam invalid requests: should return MethodNotAllowed for invalid request
Stack Traces | 0.009s run time 
Cannot read properties of undefined (reading 'statusCode')
should return QuotaExceeded when trying to PutObject in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to PutObject in a bucket with quota
Stack Traces | 0.089s run time 
ifError got unwanted exception: socket hang up
should create an encrypted bucket::KMIP backed server-side encryption should create an encrypted bucket
Stack Traces | 0.101s run time 
socket hang up
"before all" hook in "backbeat routes"::backbeat routes "before all" hook in "backbeat routes"
Stack Traces | 0.112s run time 
read ECONNRESET
should set metrics for multiPartUpload overwrite::utapi v2 metrics incoming and outgoing bytes should set metrics for multiPartUpload overwrite
Stack Traces | 0.224s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics in versioned bucket::utapi v2 metrics incoming and outgoing bytes should set metrics in versioned bucket
Stack Traces | 0.271s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for multipartUpload overwrite in a versioned bucket::utapi v2 metrics incoming and outgoing bytes should set metrics for multipartUpload overwrite in a versioned bucket
Stack Traces | 0.321s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for multipartUpload in a versioned bucket::utapi v2 metrics incoming and outgoing bytes should set metrics for multipartUpload in a versioned bucket
Stack Traces | 0.351s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for multiPartUpload::utapi v2 metrics incoming and outgoing bytes should set metrics for multiPartUpload
Stack Traces | 0.363s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for putObject and deleteObject::utapi v2 metrics incoming and outgoing bytes should set metrics for putObject and deleteObject
Stack Traces | 0.411s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for getObject::utapi v2 metrics incoming and outgoing bytes should set metrics for getObject
Stack Traces | 0.415s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for multiObjectDelete::utapi v2 metrics incoming and outgoing bytes should set metrics for multiObjectDelete
Stack Traces | 0.415s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for copyObject::utapi v2 metrics incoming and outgoing bytes should set metrics for copyObject
Stack Traces | 0.487s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should not push a metric for a filtered bucket::utapi v2 metrics incoming and outgoing bytes should not push a metric for a filtered bucket
Stack Traces | 0.509s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for multiObjectDelete in a versioned bucket::utapi v2 metrics incoming and outgoing bytes should set metrics for multiObjectDelete in a versioned bucket
Stack Traces | 0.532s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.
should set metrics for createBucket and deleteBucket::utapi v2 metrics incoming and outgoing bytes should set metrics for createBucket and deleteBucket
Stack Traces | 0.603s run time 
ifError got unwanted exception: Inaccessible host: `127.0.0.1' at port `8000'. This service may not be available in the `us-east-1' region.

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
@scality scality deleted a comment from claude bot Mar 13, 2026
@scality scality deleted a comment from claude bot Mar 13, 2026
claude[bot]
claude bot reviewed Mar 13, 2026
View reviewed changes
@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_token_bucket_2 branch from 446cb3b to ee952c3 Compare March 13, 2026 14:41
@scality scality deleted a comment from claude bot Mar 13, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

LGTM

The refactoring is clean and consistent:
- grantTokens() signature correctly updated from bucketName to resourceClass, resourceId, measure across client, tokenBucket, and all callers/tests.
- calculateInterval simplified from (limit, nodes, workers) to (limit, nodes) with consistent usage.
- WorkerTokenBucket constructor, hasCapacity(), and updateLimit() are straightforward. refillInProgress guard and finally cleanup are correct.
- Tests properly cover the new constructor params, hasCapacity(), updateLimit(), cache key format, and resource class separation.
- No async error handling issues, no double callbacks, no security concerns.

Review by Claude Code

@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_cache branch from 4b83cef to b40ff80 Compare March 18, 2026 20:47
@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_token_bucket_2 branch from ee952c3 to a68c32e Compare March 18, 2026 20:50
claude[bot]
claude bot reviewed Mar 18, 2026
View reviewed changes
claude[bot]
claude bot reviewed Mar 18, 2026
View reviewed changes
@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_cache branch from b40ff80 to 100976b Compare March 19, 2026 15:29
@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_token_bucket_2 branch from 5a26928 to e34db4d Compare March 24, 2026 18:06
@scality scality deleted a comment from claude bot Mar 24, 2026
claude[bot]
claude bot reviewed Mar 24, 2026
View reviewed changes
@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Clean refactoring. The removal of clusters from rate limit calculations, generalization of WorkerTokenBucket to support arbitrary resource classes/measures, and the API changes to grantTokens/getTokenBucket/removeTokenBucket are all consistently propagated through the codebase and tests.

- refillInProgress is no longer initialized in the WorkerTokenBucket constructor (was this.refillInProgress = false before). Works due to JS falsy semantics but should be explicitly initialized for clarity.

Review by Claude Code

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

  • Stale comment in lib/api/apiUtils/rateLimit/config.js:22: the JSDoc example comment still says limit / nodes / workers but workers has been removed from the formula. Should be limit / nodes.

Review by Claude Code

@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_token_bucket_2 branch from 7be0a34 to 4cc343b Compare March 24, 2026 18:34
@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

  • Stale comment in lib/api/apiUtils/rateLimit/config.js:22: the formula still says limit / nodes / workers but workers have been removed from the rate calculation. Should be limit / nodes.

    Review by Claude Code

anurag4DSB
anurag4DSB approved these changes Mar 25, 2026
View reviewed changes
lib/api/apiUtils/rateLimit/tokenBucket.js Outdated
const tokenBuckets = new Map();

/**
* Per-bucket token bucket for a single worker
Copy link
Copy Markdown
Contributor

@anurag4DSB anurag4DSB Mar 25, 2026

Choose a reason for hiding this comment

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

The class JSDoc still says "Per-bucket token bucket for a single worker". The class now handles any resource class (bucket, account, etc.) and the worker framing was removed from the architecture (rate limiting is now per-node). Does the comment still match the class's role?

Base automatically changed from improvement/CLDSRV-852/refactor_cache to improvement/CLDSRV-869/account_limiting March 26, 2026 16:02
@tmacro
impr(CLDSRV-852): Refactor TokenBucket class
63bdf28 
- Change constructor to accept resourceClass, resourceId, measure
- Precalculate interval for reuse in constructor
- Add hasCapacity() method
- Add updateLimit() method to encasulate limit change logic
- Remove unused mechanism to record request durations
- Simplify refill logic
@tmacro tmacro force-pushed the improvement/CLDSRV-852/refactor_token_bucket_2 branch from 4cc343b to 63bdf28 Compare March 26, 2026 16:25
@tmacro tmacro merged commit cd2c42f into improvement/CLDSRV-869/account_limiting Mar 26, 2026
5 of 6 checks passed
@tmacro tmacro deleted the improvement/CLDSRV-852/refactor_token_bucket_2 branch March 26, 2026 16:26
claude[bot]
claude bot reviewed Mar 26, 2026
View reviewed changes
lib/api/apiUtils/rateLimit/tokenBucket.js
this.bufferSize = config.rateLimiting?.tokenBucketBufferSize || 50; // Max tokens to hold
this.refillThreshold = config.rateLimiting?.tokenBucketRefillThreshold || 20; // Trigger refill when below this
this.bufferSize = config.rateLimiting?.tokenBucketBufferSize; // Max tokens to hold
this.refillThreshold = config.rateLimiting?.tokenBucketRefillThreshold; // Trigger refill when below this
Copy link
Copy Markdown

@claude claude bot Mar 26, 2026

Choose a reason for hiding this comment

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

The constructor reads bufferSize and refillThreshold with optional chaining (config.rateLimiting?.tokenBucketBufferSize), but then accesses config.rateLimiting.nodes on line 36 without optional chaining. If config.rateLimiting is undefined, this will throw a TypeError. Since rate limiting must be configured to reach this code path, drop the unnecessary optional chaining for consistency.

— Claude Code

@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026

  • tests/unit/api/apiUtils/rateLimit/refillJob.js was not updated: all 14 getTokenBucket() call sites still use the old 3-argument signature (getTokenBucket('bucket-1', { limit: 100 }, mockLog)) instead of the new 5-argument signature (getTokenBucket('bucket', 'bucket-1', 'rps', { limit: 100 }, mockLog)). These tests will pass the wrong values to the constructor (e.g. limitConfig will receive the mockLog object).
    • Update all call sites in refillJob.js to match the new API
  • lib/api/apiUtils/rateLimit/config.js line 22: stale comment still says "per-worker rate limit: limit / nodes / workers" — should say "per-node rate limit: limit / nodes"
    • Update the comment to match the new behavior
  • lib/api/apiUtils/rateLimit/tokenBucket.js lines 33-34: inconsistent optional chaining — uses config.rateLimiting? for tokenBucketBufferSize and refillThreshold but accesses config.rateLimiting.nodes without optional chaining on line 36
    • Use consistent access (drop optional chaining since rate limiting config must exist to reach this code)

Review by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claude claude[bot] claude[bot] left review comments

@anurag4DSB anurag4DSB anurag4DSB approved these changes

@BourgoisMickael BourgoisMickael BourgoisMickael approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tmacro @anurag4DSB @BourgoisMickael