-
Notifications
You must be signed in to change notification settings - Fork 19
Add v1 cortisol code #514
base: main
Are you sure you want to change the base?
Add v1 cortisol code #514
Conversation
while (nTokens == 0) { | ||
hasTokens.await(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be in a while loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, to guard against spurious wake ups.
requestCount.addAndGet(bulkDocs.size()); | ||
} | ||
|
||
private class TokenBucket { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add a comment to this class since it functions slightly different than a standard token bucket. I believe this lets you make at most nTokens concurrent take() calls every second.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
private void refill() { | ||
lock.lock(); | ||
try { | ||
nTokens = maxTokens; | ||
hasTokens.signal(); | ||
} finally { | ||
lock.unlock(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why doesn't the take() method just replenish the bucket by 1 once it's done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The work done after taking a token is dependent on the time each bulk() takes. We won't be able to guarantee a constant request rate. Some more explanation is in the next comment.
private class TokenBucket { | ||
private final Lock lock = new ReentrantLock(); | ||
private final Condition hasTokens = lock.newCondition(); | ||
private final ScheduledExecutorService ses = Executors.newSingleThreadScheduledExecutor(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can avoid having a scheduler here. Have calls to take() block, and make sure every take() refills what it took from the bucket once it's complete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take() already blocks with await() but I see what you mean here. The idea is to make sure that we make a fixed number of calls every second. If we made the call to makeBulkRequest() block and replenish the tokens once the requests are done, then we won't be able to guarantee a constant request rate. We don't want to replenish too soon(if bulk calls finish faster) or too late(if the bulk calls take longer to finish).
Fixes #:
Description of changes:
Simple load generator that ingests documents at a given rate.
Tests:
If new tests are added, how long do the new ones take to complete
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.