From 7512182b45da18be179c1e133305bc3e249049e5 Mon Sep 17 00:00:00 2001 From: andrew4699 Date: Mon, 16 Sep 2024 11:57:50 -0700 Subject: [PATCH] Change Map value to be a Future itself, use System.nanoTime --- .../polaris/service/ratelimiter/ClockImpl.java | 5 +---- .../ratelimiter/DefaultRateLimiterFactory.java | 4 ++-- .../ratelimiter/NoOpRateLimiterFactory.java | 2 +- .../service/ratelimiter/RateLimiterFactory.java | 3 +-- .../service/ratelimiter/RateLimiterFilter.java | 15 +++++---------- ...polaris.service.ratelimiter.RateLimiterFactory | 1 + .../ratelimiter/MockRateLimiterFactory.java | 2 +- 7 files changed, 12 insertions(+), 20 deletions(-) diff --git a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/ClockImpl.java b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/ClockImpl.java index 3cd0c6c56..b7585540f 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/ClockImpl.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/ClockImpl.java @@ -20,14 +20,11 @@ /** An implementation of our Clock interface using opentelemetry's Clock implementation */ public class ClockImpl implements Clock { - private final io.opentelemetry.sdk.common.Clock openTelemetryClock; - public ClockImpl() { - openTelemetryClock = io.opentelemetry.sdk.common.Clock.getDefault(); } @Override public long nanoTime() { - return openTelemetryClock.nanoTime(); + return System.nanoTime(); } } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/DefaultRateLimiterFactory.java b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/DefaultRateLimiterFactory.java index d1018b7f2..ffc1815bf 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/DefaultRateLimiterFactory.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/DefaultRateLimiterFactory.java @@ -36,10 +36,10 @@ public class DefaultRateLimiterFactory implements RateLimiterFactory { private double windowSeconds; @Override - public Future createRateLimiter(String key, Clock clock) { + public Future createRateLimiter(String key) { return CompletableFuture.supplyAsync( () -> new OpenTelemetryRateLimiter( - requestsPerSecond, requestsPerSecond * windowSeconds, clock)); + requestsPerSecond, requestsPerSecond * windowSeconds, new ClockImpl())); } } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/NoOpRateLimiterFactory.java b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/NoOpRateLimiterFactory.java index f77eee6fd..3ad276eaa 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/NoOpRateLimiterFactory.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/NoOpRateLimiterFactory.java @@ -26,7 +26,7 @@ @JsonTypeName("no-op") public class NoOpRateLimiterFactory implements RateLimiterFactory { @Override - public Future createRateLimiter(String key, Clock clock) { + public Future createRateLimiter(String key) { return CompletableFuture.supplyAsync(NoOpRateLimiter::new); } } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFactory.java b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFactory.java index 008d5c3bc..b314c655e 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFactory.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFactory.java @@ -35,8 +35,7 @@ public interface RateLimiterFactory extends Discoverable { * * @param key The rate limiting key. Rate limiters may optionally choose to discriminate their * behavior by the key. - * @param clock The clock which tells you the current time * @return a Future with the constructed RateLimiter */ - Future createRateLimiter(String key, Clock clock); + Future createRateLimiter(String key); } diff --git a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFilter.java b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFilter.java index bfd474f67..241d5a12e 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFilter.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/ratelimiter/RateLimiterFilter.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.apache.polaris.core.context.CallContext; @@ -47,7 +48,7 @@ public class RateLimiterFilter implements Filter { private static final Clock CLOCK = new ClockImpl(); private final RateLimiterConfig config; - private final Map perRealmLimiters = new ConcurrentHashMap<>(); + private final Map> perRealmLimiters = new ConcurrentHashMap<>(); public RateLimiterFilter(RateLimiterConfig config) { this.config = config; @@ -67,16 +68,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } private RateLimiter maybeBlockToGetRateLimiter(String realm) { - return perRealmLimiters.computeIfAbsent(realm, this::createRateLimiterBlocking); - } - - /** Creates a rate limiter, enforcing a timeout on how long creation can take. */ - private RateLimiter createRateLimiterBlocking(String key) { try { - return config - .getRateLimiterFactory() - .createRateLimiter(key, CLOCK) - .get(config.getConstructionTimeoutMillis(), TimeUnit.MILLISECONDS); + return perRealmLimiters.computeIfAbsent(realm, (key) -> config + .getRateLimiterFactory() + .createRateLimiter(key)).get(config.getConstructionTimeoutMillis(), TimeUnit.MILLISECONDS); } catch (InterruptedException | ExecutionException | TimeoutException e) { return getDefaultRateLimiterOnConstructionFailed(e); } diff --git a/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.ratelimiter.RateLimiterFactory b/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.ratelimiter.RateLimiterFactory index ae5038b2c..cc474a54a 100644 --- a/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.ratelimiter.RateLimiterFactory +++ b/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.ratelimiter.RateLimiterFactory @@ -18,3 +18,4 @@ # org.apache.polaris.service.ratelimiter.DefaultRateLimiterFactory +org.apache.polaris.service.ratelimiter.NoOpRateLimiterFactory diff --git a/polaris-service/src/test/java/org/apache/polaris/service/ratelimiter/MockRateLimiterFactory.java b/polaris-service/src/test/java/org/apache/polaris/service/ratelimiter/MockRateLimiterFactory.java index 5ea1312b6..8ecf409ba 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/ratelimiter/MockRateLimiterFactory.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/ratelimiter/MockRateLimiterFactory.java @@ -41,7 +41,7 @@ public class MockRateLimiterFactory implements RateLimiterFactory { public boolean neverFinishConstruction; @Override - public Future createRateLimiter(String key, Clock clock) { + public Future createRateLimiter(String key) { if (neverFinishConstruction) { // This future will never finish return new CompletableFuture<>();