Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bigtable: fix handling of unset rpc timeouts #4323

Merged
merged 1 commit into from
Jan 11, 2019
Merged

Bigtable: fix handling of unset rpc timeouts #4323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions ...ain/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ public Object getTransportCode() {
};

// Everything needed to issue an RPC
private final UnaryCallable<MutateRowsRequest, List<MutateRowsResponse>> innerCallable;
private final ApiCallContext callContext;
private MutateRowsRequest currentRequest;
@Nonnull private final UnaryCallable<MutateRowsRequest, List<MutateRowsResponse>> innerCallable;
@Nonnull private final ApiCallContext callContext;
@Nonnull private MutateRowsRequest currentRequest;

// Everything needed to build a retry request
private List<Integer> originalIndexes;
private final Set<StatusCode.Code> retryableCodes;
private final List<FailedMutation> permanentFailures;
@Nullable private List<Integer> originalIndexes;
@Nonnull private final Set<StatusCode.Code> retryableCodes;
@Nullable private final List<FailedMutation> permanentFailures;

// Parent controller
private RetryingFuture<Void> externalFuture;
Expand All @@ -135,12 +135,12 @@ public List<MutateRowsResponse> apply(Throwable throwable) {
MutateRowsAttemptCallable(
@Nonnull UnaryCallable<MutateRowsRequest, List<MutateRowsResponse>> innerCallable,
@Nonnull MutateRowsRequest originalRequest,
@Nullable ApiCallContext callContext,
@Nonnull ApiCallContext callContext,
@Nonnull Set<StatusCode.Code> retryableCodes) {
this.innerCallable = Preconditions.checkNotNull(innerCallable);
this.currentRequest = Preconditions.checkNotNull(originalRequest);
this.callContext = callContext;
this.retryableCodes = Preconditions.checkNotNull(retryableCodes);
this.innerCallable = Preconditions.checkNotNull(innerCallable, "innerCallable");
this.currentRequest = Preconditions.checkNotNull(originalRequest, "currentRequest");
this.callContext = Preconditions.checkNotNull(callContext, "callContext");
this.retryableCodes = Preconditions.checkNotNull(retryableCodes, "retryableCodes");

permanentFailures = Lists.newArrayList();
}
Expand All @@ -167,10 +167,10 @@ public Void call() {
currentRequest.getEntriesCount() > 0, "Request doesn't have any mutations to send");

// Configure the deadline
ApiCallContext currentCallContext = null;
if (callContext != null) {
ApiCallContext currentCallContext = callContext;
if (!externalFuture.getAttemptSettings().getRpcTimeout().isZero()) {
currentCallContext =
callContext.withTimeout(externalFuture.getAttemptSettings().getRpcTimeout());
currentCallContext.withTimeout(externalFuture.getAttemptSettings().getRpcTimeout());
}

// Handle concurrent cancellation
Expand Down
24 changes: 23 additions & 1 deletion ...java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,28 @@ public void singleEntrySuccessTest() throws Exception {
assertThat(innerCallable.lastRequest).isEqualTo(request);
}

@Test
public void testNoRpcTimeout() {
parentFuture.timedAttemptSettings =
parentFuture.timedAttemptSettings.toBuilder().setRpcTimeout(Duration.ZERO).build();

MutateRowsRequest request =
MutateRowsRequest.newBuilder().addEntries(Entry.getDefaultInstance()).build();

innerCallable.response.add(
MutateRowsResponse.newBuilder()
.addEntries(
MutateRowsResponse.Entry.newBuilder().setIndex(0).setStatus(OK_STATUS_PROTO))
.build());

MutateRowsAttemptCallable attemptCallable =
new MutateRowsAttemptCallable(innerCallable, request, callContext, retryCodes);
attemptCallable.setExternalFuture(parentFuture);
attemptCallable.call();

assertThat(innerCallable.lastContext.getTimeout()).isNull();
}

@Test
public void mixedTest() {
// Setup the request & response
Expand Down Expand Up @@ -340,7 +362,7 @@ public ApiFuture<List<MutateRowsResponse>> futureCall(
static class MockRetryingFuture extends AbstractApiFuture<Void> implements RetryingFuture<Void> {
ApiFuture<Void> attemptFuture;

final TimedAttemptSettings timedAttemptSettings;
TimedAttemptSettings timedAttemptSettings;

MockRetryingFuture() {
this(Duration.ofSeconds(5));
Expand Down