This repository was archived by the owner on Sep 26, 2023. It is now read-only.
Repository navigation
feat: dynamic flow control for batcher part 2 #1310
Merged
igorbernstein2
merged 17 commits into
googleapis:master
from
mutianf:dynamic_flow_control_p2
Mar 22, 2021
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
8cf73e0
feat: dynamic flow control for batcher part 2
mutianf 1906ea2
add test on construtor for code coverage
mutianf 0f62981
Update comments based on the review
mutianf c4d3337
missed some error messages in test
mutianf 692099a
Fix possible deadlock
mutianf 722d775
fix comment
mutianf 2401993
update comments
mutianf 80f70c9
update naming
mutianf 517ceb7
removing setThreshold because there's no use case for it
mutianf d374225
no need to use 2 locks on semaphore
mutianf cfe12b9
fix comment
mutianf eac770d
fix test name
mutianf c1a0543
make nonblocking semaphore not blocking
mutianf 96ef367
move limit to semaphore class
mutianf 59f29e9
add an addPermits method
mutianf a4067ce
fixing naming, don't throw on overflow and add some tests
mutianf 39fb9fe
Merge branch 'master' into dynamic_flow_control_p2
mutianf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,40 +34,90 @@ | |
|
|
||
| /** A {@link Semaphore64} that blocks until permits become available. */ | ||
| class BlockingSemaphore implements Semaphore64 { | ||
| private long currentPermits; | ||
| private long availablePermits; | ||
| private long limit; | ||
|
|
||
| private static void checkNotNegative(long l) { | ||
| Preconditions.checkArgument(l >= 0, "negative permits not allowed: %s", l); | ||
| } | ||
|
|
||
| BlockingSemaphore(long permits) { | ||
| checkNotNegative(permits); | ||
| this.currentPermits = permits; | ||
| this.availablePermits = permits; | ||
| this.limit = permits; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void release(long permits) { | ||
| checkNotNegative(permits); | ||
|
|
||
| currentPermits += permits; | ||
| // TODO: throw exceptions when the permits overflow | ||
| availablePermits = Math.min(availablePermits + permits, limit); | ||
| notifyAll(); | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized boolean acquire(long permits) { | ||
| checkNotNegative(permits); | ||
|
|
||
| boolean interrupted = false; | ||
| while (currentPermits < permits) { | ||
| while (availablePermits < permits) { | ||
| try { | ||
| wait(); | ||
| } catch (InterruptedException e) { | ||
| interrupted = true; | ||
| } | ||
| } | ||
| // TODO: if thread is interrupted, we should not grant the permits | ||
| availablePermits -= permits; | ||
|
|
||
| if (interrupted) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized boolean acquirePartial(long permits) { | ||
|
mutianf marked this conversation as resolved.
Resolved
|
||
| checkNotNegative(permits); | ||
|
|
||
| boolean interrupted = false; | ||
| // To allow individual oversized requests to be sent, clamp the requested permits to the maximum | ||
| // limit. This will allow individual large requests to be sent. Please note that this behavior | ||
| // will result in availablePermits going negative. | ||
| while (availablePermits < Math.min(limit, permits)) { | ||
| try { | ||
| wait(); | ||
| } catch (InterruptedException e) { | ||
| interrupted = true; | ||
| } | ||
| } | ||
| currentPermits -= permits; | ||
|
|
||
| if (interrupted) { | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
|
|
||
| availablePermits -= permits; | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void increasePermitLimit(long permits) { | ||
| checkNotNegative(permits); | ||
| availablePermits += permits; | ||
| limit += permits; | ||
| notifyAll(); | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void reducePermitLimit(long reduction) { | ||
| checkNotNegative(reduction); | ||
| Preconditions.checkState(limit - reduction > 0, "permit limit underflow"); | ||
| availablePermits -= reduction; | ||
| limit -= reduction; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be ok here, because the wait() in other threads are waiting for more permits to become available, while this method removes permits. So even if we call notifyAll() here, the threads are likely to go back to sleep again :). It's similar to how we don't call notifyAll() in reserve(). |
||
| } | ||
|
|
||
| @Override | ||
| public synchronized long getPermitLimit() { | ||
| return limit; | ||
| } | ||
| } | ||
169 changes: 169 additions & 0 deletions
169
gax/src/main/java/com/google/api/gax/batching/DynamicFlowControlSettings.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| /* | ||
| * Copyright 2021 Google LLC | ||
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions are | ||
| * met: | ||
| * | ||
| * * Redistributions of source code must retain the above copyright | ||
| * notice, this list of conditions and the following disclaimer. | ||
| * * Redistributions in binary form must reproduce the above | ||
| * copyright notice, this list of conditions and the following disclaimer | ||
| * in the documentation and/or other materials provided with the | ||
| * distribution. | ||
| * * Neither the name of Google LLC nor the names of its | ||
| * contributors may be used to endorse or promote products derived from | ||
| * this software without specific prior written permission. | ||
| * | ||
| * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS | ||
| * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT | ||
| * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR | ||
| * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT | ||
| * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, | ||
| * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT | ||
| * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, | ||
| * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY | ||
| * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT | ||
| * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE | ||
| * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
| package com.google.api.gax.batching; | ||
|
|
||
| import com.google.api.core.InternalApi; | ||
| import com.google.api.gax.batching.FlowController.LimitExceededBehavior; | ||
| import com.google.auto.value.AutoValue; | ||
| import com.google.common.base.Preconditions; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** Settings for dynamic flow control */ | ||
| @AutoValue | ||
| @InternalApi("For google-cloud-java client use only") | ||
|
mutianf marked this conversation as resolved.
Resolved
|
||
| public abstract class DynamicFlowControlSettings { | ||
|
|
||
| /** Number of outstanding elements that {@link FlowController} allows when it's initiated. */ | ||
| @Nullable | ||
| public abstract Long getInitialOutstandingElementCount(); | ||
|
|
||
| /** Number of outstanding bytes that {@link FlowController} allows when it's initiated. */ | ||
| @Nullable | ||
| public abstract Long getInitialOutstandingRequestBytes(); | ||
|
|
||
| /** | ||
| * Maximum number of outstanding elements {@link FlowController} allows before enforcing flow | ||
| * control. | ||
| */ | ||
| @Nullable | ||
| public abstract Long getMaxOutstandingElementCount(); | ||
|
|
||
| /** | ||
| * Maximum number of outstanding bytes {@link FlowController} allows before enforcing flow | ||
| * control. | ||
| */ | ||
| @Nullable | ||
| public abstract Long getMaxOutstandingRequestBytes(); | ||
|
|
||
| /** | ||
| * Minimum number of outstanding elements {@link FlowController} allows before enforcing flow | ||
| * control. | ||
| */ | ||
| @Nullable | ||
| public abstract Long getMinOutstandingElementCount(); | ||
|
|
||
| /** | ||
| * Minimum number of outstanding bytes {@link FlowController} allows before enforcing flow | ||
| * control. | ||
| */ | ||
| @Nullable | ||
| public abstract Long getMinOutstandingRequestBytes(); | ||
|
|
||
| /** @see FlowControlSettings#getLimitExceededBehavior() */ | ||
| public abstract LimitExceededBehavior getLimitExceededBehavior(); | ||
|
|
||
| public abstract Builder toBuilder(); | ||
|
|
||
| public static Builder newBuilder() { | ||
| return new AutoValue_DynamicFlowControlSettings.Builder() | ||
| .setLimitExceededBehavior(LimitExceededBehavior.Block); | ||
| } | ||
|
|
||
| @AutoValue.Builder | ||
| public abstract static class Builder { | ||
|
|
||
| public abstract Builder setInitialOutstandingElementCount(Long value); | ||
|
|
||
| public abstract Builder setInitialOutstandingRequestBytes(Long value); | ||
|
|
||
| public abstract Builder setMaxOutstandingElementCount(Long value); | ||
|
|
||
| public abstract Builder setMaxOutstandingRequestBytes(Long value); | ||
|
|
||
| public abstract Builder setMinOutstandingElementCount(Long value); | ||
|
|
||
| public abstract Builder setMinOutstandingRequestBytes(Long value); | ||
|
|
||
| public abstract Builder setLimitExceededBehavior(LimitExceededBehavior value); | ||
|
|
||
| abstract DynamicFlowControlSettings autoBuild(); | ||
|
|
||
| public DynamicFlowControlSettings build() { | ||
| DynamicFlowControlSettings settings = autoBuild(); | ||
|
|
||
| verifyElementCountSettings(settings); | ||
| verifyRequestBytesSettings(settings); | ||
|
|
||
| return settings; | ||
| } | ||
|
|
||
| private void verifyElementCountSettings(DynamicFlowControlSettings settings) { | ||
| boolean isEnabled = | ||
| settings.getInitialOutstandingElementCount() != null | ||
| || settings.getMinOutstandingElementCount() != null | ||
| || settings.getMaxOutstandingElementCount() != null; | ||
| if (!isEnabled) { | ||
| return; | ||
| } | ||
| Preconditions.checkState( | ||
| settings.getInitialOutstandingElementCount() != null | ||
| && settings.getMinOutstandingElementCount() != null | ||
| && settings.getMaxOutstandingElementCount() != null, | ||
| "Throttling on element count is disabled by default. To enable this setting," | ||
| + " minOutstandingElementCount, initialOutstandingElementCount, and " | ||
| + "maxOutstandingElementCount must all be set."); | ||
| Preconditions.checkState( | ||
| settings.getMinOutstandingElementCount() > 0 | ||
| && settings.getInitialOutstandingElementCount() | ||
| <= settings.getMaxOutstandingElementCount() | ||
| && settings.getInitialOutstandingElementCount() | ||
| >= settings.getMinOutstandingElementCount(), | ||
| "If throttling on element count is set, minOutstandingElementCount must be" | ||
| + " greater than 0, and minOutstandingElementCount <= " | ||
| + "initialOutstandingElementCount <= maxOutstandingElementCount"); | ||
| } | ||
|
|
||
| private void verifyRequestBytesSettings(DynamicFlowControlSettings settings) { | ||
| boolean isEnabled = | ||
| settings.getInitialOutstandingRequestBytes() != null | ||
| || settings.getMinOutstandingRequestBytes() != null | ||
| || settings.getMaxOutstandingRequestBytes() != null; | ||
| if (!isEnabled) { | ||
| return; | ||
| } | ||
| Preconditions.checkState( | ||
| settings.getInitialOutstandingRequestBytes() != null | ||
| && settings.getMinOutstandingRequestBytes() != null | ||
| && settings.getMaxOutstandingRequestBytes() != null, | ||
| "Throttling on number of bytes is disabled by default. To enable this " | ||
| + "setting, minOutstandingRequestBytes, initialOutstandingRequestBytes, and " | ||
| + "maxOutstandingRequestBytes must all be set"); | ||
| Preconditions.checkState( | ||
| settings.getMinOutstandingRequestBytes() > 0 | ||
| && settings.getInitialOutstandingRequestBytes() | ||
| <= settings.getMaxOutstandingRequestBytes() | ||
| && settings.getInitialOutstandingRequestBytes() | ||
| >= settings.getMinOutstandingRequestBytes(), | ||
| "If throttling on number of bytes is set, minOutstandingRequestBytes must " | ||
| + "be greater than 0, and minOutstandingRequestBytes <= " | ||
| + "initialOutstandingRequestBytes <= maxOutstandingRequestBytes"); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.