fhan688 commented on code in PR #20189:
URL: https://github.com/apache/hudi/pull/20189#discussion_r4229313730
##########
hudi-common/src/test/java/org/apache/hudi/common/util/TestRatelimiter.java:
##########
@@ -33,25 +44,91 @@ public class TestRatelimiter {
@Test
public void testRateLimiterWithNoThrottling() throws InterruptedException {
RateLimiter limiter = RateLimiter.create(1000, TimeUnit.SECONDS);
- long start = System.currentTimeMillis();
- assertEquals(true, limiter.tryAcquire(1000));
- // Sleep to represent some operation
- Thread.sleep(500);
- long end = System.currentTimeMillis();
- // With a large permit limit, there shouldn't be any throttling of
operations
- assertTrue((end - start) < TimeUnit.SECONDS.toMillis(2));
+ try {
+ long start = System.currentTimeMillis();
+ assertEquals(true, limiter.tryAcquire(1000));
+ // Sleep to represent some operation
+ Thread.sleep(500);
+ long end = System.currentTimeMillis();
+ // With a large permit limit, there shouldn't be any throttling of
operations
+ assertTrue((end - start) < TimeUnit.SECONDS.toMillis(2));
+ } finally {
+ limiter.stop();
+ }
}
@Test
public void testRateLimiterWithThrottling() throws InterruptedException {
RateLimiter limiter = RateLimiter.create(100, TimeUnit.SECONDS);
- long start = System.currentTimeMillis();
- assertEquals(true, limiter.tryAcquire(400));
- // Sleep to represent some operation
- Thread.sleep(500);
- long end = System.currentTimeMillis();
- // As size of operations is more than the maximum permits per second,
- // whole execution should be greater than 1 second
- assertTrue((end - start) >= TimeUnit.SECONDS.toMillis(2));
+ try {
+ long start = System.currentTimeMillis();
+ assertEquals(true, limiter.tryAcquire(400));
+ // Sleep to represent some operation
+ Thread.sleep(500);
+ long end = System.currentTimeMillis();
+ // As size of operations is more than the maximum permits per second,
+ // whole execution should be greater than 1 second
+ assertTrue((end - start) >= TimeUnit.SECONDS.toMillis(2));
+ } finally {
+ limiter.stop();
+ }
+ }
+
+ @Test
+ public void testCustomReleasePeriod() {
+ ScheduledExecutorService scheduler = mock(ScheduledExecutorService.class);
+
+ RateLimiter limiter = RateLimiter.create(1, 100, TimeUnit.MILLISECONDS,
scheduler);
+ ArgumentCaptor<Runnable> refillTask =
ArgumentCaptor.forClass(Runnable.class);
+ try {
+ verify(scheduler).scheduleAtFixedRate(
+ refillTask.capture(), eq(100L), eq(100L), eq(TimeUnit.MILLISECONDS));
+ assertTrue(limiter.acquire());
+ refillTask.getValue().run();
+ assertTrue(limiter.acquire());
+ } finally {
+ limiter.stop();
+ }
+ verify(scheduler).shutdownNow();
+ }
+
+ @Test
+ public void testInvalidConfigurationIsRejected() {
+ assertThrows(IllegalArgumentException.class, () -> RateLimiter.create(0,
TimeUnit.SECONDS));
+ assertThrows(IllegalArgumentException.class, () -> RateLimiter.create(1,
0, TimeUnit.SECONDS));
+ assertThrows(IllegalArgumentException.class, () -> RateLimiter.create(1,
1, null));
+ }
+
+ @Test
+ public void testInvalidAcquireRequestIsRejected() {
+ RateLimiter limiter = RateLimiter.create(1, 1, TimeUnit.DAYS);
+ try {
+ assertThrows(IllegalArgumentException.class, () -> limiter.acquire(0));
+ assertThrows(IllegalArgumentException.class, () -> limiter.acquire(2));
+ } finally {
+ limiter.stop();
+ }
+ }
+
+ @Test
+ public void testStopIsIdempotentAndUnblocksAcquire() throws Exception {
+ RateLimiter limiter = RateLimiter.create(1, 1, TimeUnit.DAYS);
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ try {
+ assertTrue(limiter.acquire());
+ Future<Boolean> blockedAcquire = executor.submit(() ->
limiter.acquire());
+
+ limiter.stop();
Review Comment:
Thanks for pointing this out. I’ve made the test deterministic by adding a
test-only callback that signals a `CountDownLatch` after the permit acquisition
fails and enters the waiting path. The test now waits for this signal before
calling `stop()`, ensuring it actually verifies that `stop()` unblocks an
ongoing `acquire()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
