Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

Commit e18740b

Browse files
chingor13chingor13
authored
Merge branch 'master' into release-please/branches/master
2 parents 7f00de0 + 4c63a0c commit e18740b

11 files changed

Lines changed: 264 additions & 74 deletions

File tree

‎.github/workflows/ci.yaml‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,46 @@ jobs:
2525
name: actions ${{ matrix.java }}
2626
bazel:
2727
runs-on: ubuntu-latest
28+
container: gcr.io/gapic-images/googleapis-bazel:20210105
2829
steps:
2930
- uses: actions/checkout@v2
3031
- uses: actions/setup-java@v1
3132
with:
3233
java-version: 8
3334
- run: java -version
34-
- name: Install Bazel
35+
36+
- name: Bazel File Cache Setup
37+
id: cache-bazel
38+
uses: actions/cache@v2
39+
with:
40+
path: ~/.cache/bazel
41+
key: ${{ runner.os }}-bazel-20210105-${{ secrets.CACHE_VERSION }}
42+
43+
- name: Bazel Cache Not Found
44+
if: steps.cache-bazel.outputs.cache-hit != 'true'
3545
run: |
36-
wget -q "https://github.com/bazelbuild/bazel/releases/download/$BAZEL_VERSION/$BAZEL_BINARY"
37-
wget -q "https://github.com/bazelbuild/bazel/releases/download/$BAZEL_VERSION/$BAZEL_BINARY.sha256"
38-
sha256sum -c "$BAZEL_BINARY.sha256"
39-
sudo dpkg -i "$BAZEL_BINARY"
40-
env:
41-
BAZEL_VERSION: 3.5.0
42-
BAZEL_BINARY: bazel_3.5.0-linux-x86_64.deb
46+
echo "No cache found."
47+
- name: Bazel Cache Found
48+
if: steps.cache-bazel.outputs.cache-hit == 'true'
49+
run: |
50+
echo -n "Cache found. Cache size: "
51+
du -sh ~/.cache/bazel
52+
echo "If the cache seems broken, update the CACHE_VERSION secret in"
53+
echo "https://github.com/googleapis/googleapis-discovery/settings/secrets/actions"
54+
echo "(use any random string, any GUID will work)"
55+
echo "and it will start over with a clean cache."
56+
echo "The old one will disappear after 7 days."
57+
4358
- name: Run bazel tests
4459
run: bazel --batch test //... --noshow_progress --test_output=errors
60+
61+
- uses: actions/upload-artifact@v2
62+
if: ${{ failure() }}
63+
with:
64+
name: test-artifacts
65+
path: ~/.cache/bazel/*/*/*/gax-java/bazel-out/*/testlogs/*
66+
retention-days: 5
67+
4568
- name: coverage
4669
uses: codecov/codecov-action@v1
4770
with:

‎dependencies.properties‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,18 @@ maven.com_google_auto_value_auto_value=com.google.auto.value:auto-value:1.4
6464
maven.com_google_auto_value_auto_value_annotations=com.google.auto.value:auto-value-annotations:1.7.4
6565
maven.com_google_api_api_common=com.google.api:api-common:1.10.1
6666
maven.org_threeten_threetenbp=org.threeten:threetenbp:1.5.0
67-
maven.com_google_api_grpc_grpc_google_iam_v1=com.google.api.grpc:grpc-google-iam-v1:1.0.7
68-
maven.com_google_api_grpc_proto_google_iam_v1=com.google.api.grpc:proto-google-iam-v1:1.0.7
67+
maven.com_google_api_grpc_grpc_google_iam_v1=com.google.api.grpc:grpc-google-iam-v1:1.0.9
68+
maven.com_google_api_grpc_proto_google_iam_v1=com.google.api.grpc:proto-google-iam-v1:1.0.9
6969
maven.com_google_http_client_google_http_client=com.google.http-client:google-http-client:1.38.1
7070
maven.com_google_http_client_google_http_client_gson=com.google.http-client:google-http-client-gson:1.38.1
7171
maven.org_codehaus_mojo_animal_sniffer_annotations=org.codehaus.mojo:animal-sniffer-annotations:1.18
7272
maven.javax_annotation_javax_annotation_api=javax.annotation:javax.annotation-api:1.3.2
7373

7474
# Testing maven artifacts
75-
maven.junit_junit=junit:junit:4.13.1
75+
maven.junit_junit=junit:junit:4.13.2
7676
maven.org_mockito_mockito_core=org.mockito:mockito-core:2.28.2
7777
maven.org_hamcrest_hamcrest_core=org.hamcrest:hamcrest-core:1.3
78-
maven.com_google_truth_truth=com.google.truth:truth:1.0.1
78+
maven.com_google_truth_truth=com.google.truth:truth:1.1.2
7979
maven.com_googlecode_java_diff_utils_diffutils=com.googlecode.java-diff-utils:diffutils:1.3.0
8080
maven.net_bytebuddy_byte_buddy=net.bytebuddy:byte-buddy:1.9.7
8181
maven.org_objenesis_objenesis=org.objenesis:objenesis:2.6

‎gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
package com.google.api.gax.grpc;
3131

3232
import com.google.api.core.BetaApi;
33-
import com.google.api.core.InternalExtensionOnly;
3433
import com.google.api.gax.rpc.ApiCallContext;
3534
import com.google.api.gax.rpc.TransportChannel;
3635
import com.google.api.gax.rpc.internal.Headers;
@@ -62,7 +61,6 @@
6261
* and thread safety of the arguments solely depends on the arguments themselves.
6362
*/
6463
@BetaApi("Reference ApiCallContext instead - this class is likely to experience breaking changes")
65-
@InternalExtensionOnly
6664
public final class GrpcCallContext implements ApiCallContext {
6765
static final CallOptions.Key<ApiTracer> TRACER_KEY = Key.create("gax.tracer");
6866

‎gax-httpjson/src/main/java/com/google/api/gax/httpjson/ApiMessageHttpRequestFormatter.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ ApiMessageHttpRequestFormatter.Builder<RequestT> newBuilder() {
7575
return new ApiMessageHttpRequestFormatter.Builder<>();
7676
}
7777

78+
@SuppressWarnings("unchecked")
7879
@Override
7980
public Map<String, List<String>> getQueryParamNames(RequestT apiMessage) {
8081
Set<String> paramNames = getQueryParamNames();

‎gax-httpjson/src/main/java/com/google/api/gax/httpjson/ApiMethodDescriptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public abstract class ApiMethodDescriptor<RequestT, ResponseT> {
5050
public abstract String getHttpMethod();
5151

5252
public static <RequestT, ResponseT> Builder<RequestT, ResponseT> newBuilder() {
53-
return new AutoValue_ApiMethodDescriptor.Builder();
53+
return new AutoValue_ApiMethodDescriptor.Builder<RequestT, ResponseT>();
5454
}
5555

5656
@AutoValue.Builder

‎gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
package com.google.api.gax.httpjson;
3131

3232
import com.google.api.core.BetaApi;
33-
import com.google.api.core.InternalExtensionOnly;
3433
import com.google.api.gax.rpc.ApiCallContext;
3534
import com.google.api.gax.rpc.TransportChannel;
3635
import com.google.api.gax.rpc.internal.Headers;
@@ -56,7 +55,6 @@
5655
* arguments solely depends on the arguments themselves.
5756
*/
5857
@BetaApi
59-
@InternalExtensionOnly
6058
public final class HttpJsonCallContext implements ApiCallContext {
6159
private final HttpJsonChannel channel;
6260
private final Duration timeout;

‎gax/src/main/java/com/google/api/gax/batching/BatcherImpl.java‎

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
import com.google.api.core.BetaApi;
3838
import com.google.api.core.InternalApi;
3939
import com.google.api.core.SettableApiFuture;
40+
import com.google.api.gax.batching.FlowController.FlowControlException;
41+
import com.google.api.gax.batching.FlowController.FlowControlRuntimeException;
42+
import com.google.api.gax.batching.FlowController.LimitExceededBehavior;
4043
import com.google.api.gax.rpc.UnaryCallable;
4144
import com.google.common.annotations.VisibleForTesting;
4245
import com.google.common.base.Preconditions;
@@ -55,6 +58,7 @@
5558
import java.util.concurrent.atomic.AtomicInteger;
5659
import java.util.logging.Level;
5760
import java.util.logging.Logger;
61+
import javax.annotation.Nullable;
5862

5963
/**
6064
* Queues up the elements until {@link #flush()} is called; once batching is over, returned future
@@ -87,13 +91,14 @@ public class BatcherImpl<ElementT, ElementResultT, RequestT, ResponseT>
8791
private final Future<?> scheduledFuture;
8892
private volatile boolean isClosed = false;
8993
private final BatcherStats batcherStats = new BatcherStats();
94+
private final FlowController flowController;
9095

9196
/**
9297
* @param batchingDescriptor a {@link BatchingDescriptor} for transforming individual elements
93-
* into wrappers request and response.
94-
* @param unaryCallable a {@link UnaryCallable} object.
95-
* @param prototype a {@link RequestT} object.
96-
* @param batchingSettings a {@link BatchingSettings} with configuration of thresholds.
98+
* into wrappers request and response
99+
* @param unaryCallable a {@link UnaryCallable} object
100+
* @param prototype a {@link RequestT} object
101+
* @param batchingSettings a {@link BatchingSettings} with configuration of thresholds
97102
*/
98103
public BatcherImpl(
99104
BatchingDescriptor<ElementT, ElementResultT, RequestT, ResponseT> batchingDescriptor,
@@ -102,15 +107,56 @@ public BatcherImpl(
102107
BatchingSettings batchingSettings,
103108
ScheduledExecutorService executor) {
104109

110+
this(batchingDescriptor, unaryCallable, prototype, batchingSettings, executor, null);
111+
}
112+
113+
/**
114+
* @param batchingDescriptor a {@link BatchingDescriptor} for transforming individual elements
115+
* into wrappers request and response
116+
* @param unaryCallable a {@link UnaryCallable} object
117+
* @param prototype a {@link RequestT} object
118+
* @param batchingSettings a {@link BatchingSettings} with configuration of thresholds
119+
* @param flowController a {@link FlowController} for throttling requests. If it's null, create a
120+
* {@link FlowController} object from {@link BatchingSettings#getFlowControlSettings()}.
121+
*/
122+
public BatcherImpl(
123+
BatchingDescriptor<ElementT, ElementResultT, RequestT, ResponseT> batchingDescriptor,
124+
UnaryCallable<RequestT, ResponseT> unaryCallable,
125+
RequestT prototype,
126+
BatchingSettings batchingSettings,
127+
ScheduledExecutorService executor,
128+
@Nullable FlowController flowController) {
129+
105130
this.batchingDescriptor =
106131
Preconditions.checkNotNull(batchingDescriptor, "batching descriptor cannot be null");
107132
this.unaryCallable = Preconditions.checkNotNull(unaryCallable, "callable cannot be null");
108133
this.prototype = Preconditions.checkNotNull(prototype, "request prototype cannot be null");
109134
this.batchingSettings =
110135
Preconditions.checkNotNull(batchingSettings, "batching setting cannot be null");
111136
Preconditions.checkNotNull(executor, "executor cannot be null");
137+
if (flowController == null) {
138+
flowController = new FlowController(batchingSettings.getFlowControlSettings());
139+
}
140+
// If throttling is enabled, make sure flow control limits are greater or equal to batch sizes
141+
// to avoid deadlocking
142+
if (flowController.getLimitExceededBehavior() != LimitExceededBehavior.Ignore) {
143+
Preconditions.checkArgument(
144+
flowController.getMaxOutstandingElementCount() == null
145+
|| batchingSettings.getElementCountThreshold() == null
146+
|| flowController.getMaxOutstandingElementCount()
147+
>= batchingSettings.getElementCountThreshold(),
148+
"If throttling and batching on element count are enabled, FlowController"
149+
+ "#maxOutstandingElementCount must be greater or equal to elementCountThreshold");
150+
Preconditions.checkArgument(
151+
flowController.getMaxOutstandingRequestBytes() == null
152+
|| batchingSettings.getRequestByteThreshold() == null
153+
|| flowController.getMaxOutstandingRequestBytes()
154+
>= batchingSettings.getRequestByteThreshold(),
155+
"If throttling and batching on request bytes are enabled, FlowController"
156+
+ "#maxOutstandingRequestBytes must be greater or equal to requestByteThreshold");
157+
}
158+
this.flowController = flowController;
112159
currentOpenBatch = new Batch<>(prototype, batchingDescriptor, batchingSettings, batcherStats);
113-
114160
if (batchingSettings.getDelayThreshold() != null) {
115161
long delay = batchingSettings.getDelayThreshold().toMillis();
116162
PushCurrentBatchRunnable<ElementT, ElementResultT, RequestT, ResponseT> runnable =
@@ -127,8 +173,29 @@ public BatcherImpl(
127173
@Override
128174
public ApiFuture<ElementResultT> add(ElementT element) {
129175
Preconditions.checkState(!isClosed, "Cannot add elements on a closed batcher");
130-
SettableApiFuture<ElementResultT> result = SettableApiFuture.create();
176+
// This is not the optimal way of throttling. It does not send out partial batches, which
177+
// means that the Batcher might not use up all the resources allowed by FlowController.
178+
// The more efficient implementation should look like:
179+
// if (!flowController.tryReserve(1, bytes)) {
180+
// sendOutstanding();
181+
// reserve(1, bytes);
182+
// }
183+
// where tryReserve() will return false if there isn't enough resources, or reserve and return
184+
// true.
185+
// However, with the current FlowController implementation, adding a tryReserve() could be
186+
// confusing. FlowController will end up having 3 different reserve behaviors: blocking,
187+
// non blocking and try reserve. And we'll also need to add a tryAcquire() to the Semaphore64
188+
// class, which made it seem unnecessary to have blocking and non-blocking semaphore
189+
// implementations. Some refactoring may be needed for the optimized implementation. So we'll
190+
// defer it till we decide on if refactoring FlowController is necessary.
191+
try {
192+
flowController.reserve(1, batchingDescriptor.countBytes(element));
193+
} catch (FlowControlException e) {
194+
// This exception will only be thrown if the FlowController is set to ThrowException behavior
195+
throw FlowControlRuntimeException.fromFlowControlException(e);
196+
}
131197

198+
SettableApiFuture<ElementResultT> result = SettableApiFuture.create();
132199
synchronized (elementLock) {
133200
currentOpenBatch.add(element, result);
134201
}
@@ -169,6 +236,7 @@ public void sendOutstanding() {
169236
@Override
170237
public void onSuccess(ResponseT response) {
171238
try {
239+
flowController.release(accumulatedBatch.elementCounter, accumulatedBatch.byteCounter);
172240
accumulatedBatch.onBatchSuccess(response);
173241
} finally {
174242
onBatchCompletion();
@@ -178,6 +246,7 @@ public void onSuccess(ResponseT response) {
178246
@Override
179247
public void onFailure(Throwable throwable) {
180248
try {
249+
flowController.release(accumulatedBatch.elementCounter, accumulatedBatch.byteCounter);
181250
accumulatedBatch.onBatchFailure(throwable);
182251
} finally {
183252
onBatchCompletion();
@@ -224,6 +293,12 @@ public void close() throws InterruptedException {
224293
}
225294
}
226295

296+
/** Package-private for use in testing. */
297+
@VisibleForTesting
298+
FlowController getFlowController() {
299+
return flowController;
300+
}
301+
227302
/**
228303
* This class represent one logical Batch. It accumulates all the elements and their corresponding
229304
* future results for one batch.

‎gax/src/main/java/com/google/api/gax/batching/FlowController.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
package com.google.api.gax.batching;
3131

3232
import com.google.api.core.BetaApi;
33+
import com.google.api.core.InternalApi;
3334
import com.google.common.base.Preconditions;
3435
import javax.annotation.Nullable;
3536

@@ -143,9 +144,10 @@ public enum LimitExceededBehavior {
143144
@Nullable private final Semaphore64 outstandingByteCount;
144145
@Nullable private final Long maxOutstandingElementCount;
145146
@Nullable private final Long maxOutstandingRequestBytes;
147+
private final LimitExceededBehavior limitExceededBehavior;
146148

147149
public FlowController(FlowControlSettings settings) {
148-
boolean failOnLimits;
150+
this.limitExceededBehavior = settings.getLimitExceededBehavior();
149151
switch (settings.getLimitExceededBehavior()) {
150152
case ThrowException:
151153
case Block:
@@ -216,4 +218,20 @@ public void release(long elements, long bytes) {
216218
outstandingByteCount.release(permitsToReturn);
217219
}
218220
}
221+
222+
LimitExceededBehavior getLimitExceededBehavior() {
223+
return limitExceededBehavior;
224+
}
225+
226+
@InternalApi("For internal use by google-cloud-java clients only")
227+
@Nullable
228+
public Long getMaxOutstandingElementCount() {
229+
return maxOutstandingElementCount;
230+
}
231+
232+
@InternalApi("For internal use by google-cloud-java clients only")
233+
@Nullable
234+
public Long getMaxOutstandingRequestBytes() {
235+
return maxOutstandingRequestBytes;
236+
}
219237
}

‎gax/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) {
187187

188188
// Injecting Random is not possible here, as Random does not provide nextLong(long bound) method
189189
protected long nextRandomLong(long bound) {
190-
return bound > 0 && globalSettings.isJittered()
190+
return bound > 0 && globalSettings.isJittered() // Jitter check needed for testing purposes.
191191
? ThreadLocalRandom.current().nextLong(bound)
192192
: bound;
193193
}

‎gax/src/main/java/com/google/api/gax/retrying/RetrySettings.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
package com.google.api.gax.retrying;
3131

3232
import com.google.auto.value.AutoValue;
33+
import com.google.common.annotations.VisibleForTesting;
3334
import java.io.Serializable;
3435
import org.threeten.bp.Duration;
3536

@@ -112,7 +113,11 @@ public abstract class RetrySettings implements Serializable {
112113
* <pre>{@code actualDelay = rand_between(0, min(maxRetryDelay, delay))}</pre>
113114
*
114115
* The default value is {@code true}.
116+
*
117+
* @deprecated Retries always jitter.
115118
*/
119+
@Deprecated
120+
@VisibleForTesting
116121
public abstract boolean isJittered();
117122

118123
/**
@@ -194,13 +199,17 @@ public abstract static class Builder {
194199
public abstract Builder setMaxAttempts(int maxAttempts);
195200

196201
/**
197-
* Jitter determines if the delay time should be randomized. In most cases, if jitter is set to
198-
* {@code true} the actual delay time is calculated in the following way:
202+
* Jitter determines if the delay time should be randomized. If jitter is set to {@code true}
203+
* the actual delay time is calculated in the following way:
199204
*
200205
* <pre>{@code actualDelay = rand_between(0, min(maxRetryDelay, exponentialDelay))}</pre>
201206
*
202-
* The default value is {@code true}.
207+
* The default value is {@code true}, and this method will be a no-op soon.
208+
*
209+
* @deprecated Retries always jitter.
203210
*/
211+
@Deprecated
212+
@VisibleForTesting
204213
public abstract Builder setJittered(boolean jittered);
205214

206215
/**

0 commit comments

Comments
 (0)