Skip to content

Do not allow warmupPeriod set as zero - #3724

Open
liufuyang wants to merge 1 commit into
google:masterfrom
liufuyang:patch-1
Open

Do not allow warmupPeriod set as zero#3724
liufuyang wants to merge 1 commit into
google:masterfrom
liufuyang:patch-1

Conversation

@liufuyang

@liufuyang liufuyang commented Nov 28, 2019

Copy link
Copy Markdown

In order to solve issues like #2730
We encountered this "crazy" issue in production that when setting warmupPeriod=0, the ratelimiter changed the rate from 1 per second to millions (if not billions) per second.

Or simply see this issue by running following code as a test:

  @Test
  public void t() {
    RateLimiter rateLimiter = RateLimiter.create(1, 0, TimeUnit.SECONDS);

    while (true) {
      rateLimiter.acquire();
      System.out.println("Done");
    }
  }

When setting warmupPeriod=0 will trigger some downstream slope calculation with some value divide 0: https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/SmoothRateLimiter.java#L229
image

@liufuyang

Copy link
Copy Markdown
Author

@cpovirk Perhaps you can give a review of this?

@liufuyang

Copy link
Copy Markdown
Author

@cgdecker Perhaps you can take a look as well? :)

@liufuyang

Copy link
Copy Markdown
Author

@ronshapiro @kluever Pinging you two here as I saw you changed this ratelimiter recently 😄

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants