Replace custom token-bucket implementation with Guava's `RateLimiter` by snazy · Pull Request #3507 · apache/polaris

snazy · 2026-01-22T14:55:24Z

Addresses the issues discussed on the dev mailing-list discussion, opting to use Guava as the easiest replacement here.

adutra

LGTM - simple and easy 😄

dimas-b

The approach LGTM 👍 just one config concern.

dimas-b · 2026-01-22T15:30:23Z

...e/service/src/main/java/org/apache/polaris/service/ratelimiter/TokenBucketConfiguration.java

+   */
  long requestsPerSecond();

-  Duration window();


This could be a breaking change if older deployments defined a config value for it in application.properties (which is introspected and validated by Quarkus, IIRC). A missing "receiver" for those configs could be a runtime error (and config is under strict guarantees in our standing evolution guidelines).

Could we keep a deprecated (in code and in CHANGELOG) receiver just to improve backward compatibility?

Re-added the window option as "deprecated for removal" and added a note to the CHANGELOG.

MonkeyCanCode · 2026-01-23T00:27:53Z

helm/polaris/README.md

 | rateLimiter.tokenBucket | object | `{"requestsPerSecond":9999,"type":"default","window":"PT10S"}` | The configuration for the default rate limiter, which uses the token bucket algorithm with one bucket per realm. |
 | rateLimiter.tokenBucket.requestsPerSecond | int | `9999` | The maximum number of requests per second allowed for each realm. |
 | rateLimiter.tokenBucket.type | string | `"default"` | The type of the token bucket rate limiter. Only the default type is supported out of the box. |
-| rateLimiter.tokenBucket.window | string | `"PT10S"` | The time window. |


As this is not a backward compatible change, should we update CHANGELOG.md as well?

Added notes to the CHANGELOG

MonkeyCanCode · 2026-01-23T00:31:06Z

helm/polaris/templates/configmap.yaml

    {{- if ne .Values.rateLimiter.type "no-op" -}}
    {{- $_ = set $map "polaris.rate-limiter.token-bucket.type" .Values.rateLimiter.tokenBucket.type -}}
    {{- $_ = set $map "polaris.rate-limiter.token-bucket.requests-per-second" .Values.rateLimiter.tokenBucket.requestsPerSecond -}}
-    {{- $_ = set $map "polaris.rate-limiter.token-bucket.window" .Values.rateLimiter.tokenBucket.window -}}


We will also need to update https://github.com/apache/polaris/blob/main/helm/polaris/tests/configmap_test.yaml:

./helm/polaris/tests/configmap_test.yaml: - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.rate-limiter.token-bucket.window=PT10S" } ./helm/polaris/tests/configmap_test.yaml: - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.rate-limiter.token-bucket.window=PT5S" }

Those are already removed (see below)

I missed to remove the window setting right above in configmap_test tho

snazy · 2026-01-23T15:53:03Z

Rebased due to a conflict w/ #3503

dimas-b · 2026-01-23T16:08:25Z

CHANGELOG.md

 ### Breaking changes

 - The (Before/After)CommitTableEvent has been removed.
+- The configuration option `polaris.rate-limiter.token-bucket.window` is no longer supported and should be removed.


Should this be under Deprecations for now? We've not removed the "receiver" for this config, only deprecated 🤔

I thought about that, too.
However, people should remove it now (or better: with the next release) to be safe for the release after that.
Sure, it's not really breaking, however, the parameter is no longer used - could be a interpreted as "a soft breaking change" (not sure there's a better term for that).

I think I agree with @dimas-b, the Deprecations section seems like a better fit for this announcement.

When the property is fully removed, then we would put a note under this section (since that would be a hard breaking change).

I see that this is what we did, e.g. for the EclipseLink phase out.

okay, moved

Addresses the issues discussed on the dev mailing-list discussion https://lists.apache.org/thread/gkyw7m4fcbjbzhcrlrp4kcq5lr05r0m4, opting to use Guava as the easiest replacement here.

snazy · 2026-01-26T14:33:20Z

Had to rebase again

dimas-b

LGTM 👍 Nice improvement as discussed on dev

snazy · 2026-01-26T15:18:05Z

Thanks for the reviews :)

…apache#3507) Addresses the issues discussed on the dev mailing-list discussion https://lists.apache.org/thread/gkyw7m4fcbjbzhcrlrp4kcq5lr05r0m4, opting to use Guava as the easiest replacement here.

* Replace custom token-bucket implementation with Guava's `RateLimiter` (apache#3507) Addresses the issues discussed on the dev mailing-list discussion https://lists.apache.org/thread/gkyw7m4fcbjbzhcrlrp4kcq5lr05r0m4, opting to use Guava as the easiest replacement here. * Move idempotency_records schema to v4 and add H2 support (apache#3386) * Move idempotency_records schema to v4 and add H2 support * address comments and fix test failures * fix format * add comment to resource_id * (nit): Getting started examples with mc/s5cmd to aws cli (apache#3526) * Switch mc/s3cmd to aws cli * Switch mc/s3cmd to aws cli * Add support for no KMS with s3-compatible backend (apache#3501) * chore(deps): update amazon/aws-cli docker tag to v2.33.7 (apache#3558) * Update doc for helm around rateLimiter (apache#3562) * Disable renoavte update for python version (apache#3560) * Fix the Keycloak getting-started example for 26.5+ (apache#3568) The example was failing because Keycloak 26.5 introduced stricter validation rules for session lifespan and timeout. * NoSQL: Add to runtime-service (apache#3396) * NoSQL: Add to runtime-service This change adds the NoSQL persistence to polaris-runtime-service. * chore(deps): update amazon/aws-cli docker tag to v2.33.8 (apache#3575) * Add spark sql integration test for Hudi (apache#3194) * Fix ozone getting started example (apache#3574) * Fix Ozone getting started example * Fix Ozone getting started example * Change AWS CLI image to weekly (apache#3578) * fix(deps): update dependency com.diffplug.spotless:spotless-plugin-gradle to v8.2.1 (apache#3576) * chore(deps): update registry.access.redhat.com/ubi9/openjdk-21-runtime docker tag to v1.24-2.1769108682 (apache#3588) * removed references of BEFORE/AFTER_COMMIT_VIEW (apache#3554) * nits - post-merge fixes * Last merged commit 2b0ca21 --------- Co-authored-by: Huaxin Gao <huaxin.gao11@gmail.com> Co-authored-by: Yong Zheng <yongzheng0809@gmail.com> Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: Alexandre Dutra <adutra@apache.org> Co-authored-by: Rahil C <32500120+rahil-c@users.noreply.github.com> Co-authored-by: Innocent Djiofack <djiofack007@gmail.com>

github-project-automation bot added this to Basic Kanban Board Jan 22, 2026

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jan 22, 2026

adutra previously approved these changes Jan 22, 2026

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 22, 2026

snazy mentioned this pull request Jan 22, 2026

[POC] Replace custom token bucket with Quarkus Bucket4J extension #3506

Closed

6 tasks

dimas-b reviewed Jan 22, 2026

View reviewed changes

MonkeyCanCode reviewed Jan 23, 2026

View reviewed changes

snazy dismissed adutra’s stale review via 76b3a32 January 23, 2026 12:11

snazy force-pushed the token-bucket-guava branch from d133046 to 5742b60 Compare January 23, 2026 15:52

dimas-b reviewed Jan 23, 2026

View reviewed changes

snazy added 5 commits January 26, 2026 15:32

Replace custom token-bucket implementation with Guava's RateLimiter

97c836e

Addresses the issues discussed on the dev mailing-list discussion https://lists.apache.org/thread/gkyw7m4fcbjbzhcrlrp4kcq5lr05r0m4, opting to use Guava as the easiest replacement here.

helm test - missed option

3ab9f94

re-add window as deprecated-for-removal

558af04

changelog

16cabcd

fix

9b299e1

snazy force-pushed the token-bucket-guava branch from 5742b60 to 9b299e1 Compare January 26, 2026 14:33

dimas-b approved these changes Jan 26, 2026

View reviewed changes

adutra approved these changes Jan 26, 2026

View reviewed changes

snazy merged commit 1d7903c into apache:main Jan 26, 2026
15 checks passed

snazy deleted the token-bucket-guava branch January 26, 2026 15:18

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jan 26, 2026

Conversation

snazy commented Jan 22, 2026

Uh oh!

adutra left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snazy commented Jan 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snazy commented Jan 26, 2026

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

snazy commented Jan 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dimas-b Jan 22, 2026 •

edited

Loading