Skip to content

Commit

Permalink
feat: add toString to futures returned by operations (#3140)
Browse files Browse the repository at this point in the history
Sometimes an operation can get stuck indefinitely. The underlying
reasons can vary significantly:
- the underlying attempt rpc can get stuck due to a bug in grpc (ie
grpc/grpc-java#11026)
- the operation can get stuck in layers above gax:
googleapis/java-bigtable#1939
- or it can get stuck in gax itself (dont have a pointer handy)

Guava futures provide some observability for ListenableFutures, but in
creating the custom ApiFutures in gax, we lose that functionality. This
PR sprinkles a few to toString to allow callers to inspect the internal
state of the operation. For example with these changes, the toString()
of the future returned from bigtableDataClient.mutateRows() changes from

> TransformFuture@652ce654[status=PENDING,
info=[inputFuture=[com.google.api.core.ApiFutureToListenableFuture@522ba524],
function=[com.google.api.core.ApiFutures$ApiFunctionToGuavaFunction@29c5ee1d]]]

to
>
ListenableFutureToApiFuture{delegate=TransformFuture@7ac9af2a[status=PENDING,
info=[inputFuture=[ApiFutureToListenableFuture{apiFuture=CallbackChainRetryingFuture{super=com.google.api.gax.retrying.CallbackChainRetryingFuture@7bb004b8[status=PENDING],
latestCompletedAttemptResult=null, attemptResult=null,
attemptSettings=TimedAttemptSettings{globalSettings=RetrySettings{totalTimeout=PT10M,
initialRetryDelay=PT0.01S, retryDelayMultiplier=2.0, maxRetryDelay=PT1M,
maxAttempts=0, jittered=true, initialRpcTimeout=PT1M,
rpcTimeoutMultiplier=1.0, maxRpcTimeout=PT1M}, retryDelay=PT0S,
rpcTimeout=PT1M, randomizedRetryDelay=PT0S, attemptCount=0,
overallAttemptCount=0, firstAttemptStartTimeNanos=635709620001791}}}],
function=[com.google.api.core.ApiFutures$ApiFunctionToGuavaFunction@652ce654]]]}

This allows us to reason about whats stuck. I'm working another PR that
will add a close(timeout) to the Batcher that will use this
functionality to identify why batcher.close() timed out
  • Loading branch information
igorbernstein2 authored Aug 30, 2024
1 parent cb24fdb commit afecb8c
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/
package com.google.api.core;

import com.google.common.base.MoreObjects;
import com.google.common.util.concurrent.ListenableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -74,4 +75,11 @@ public V get(long l, TimeUnit timeUnit)
throws InterruptedException, ExecutionException, TimeoutException {
return apiFuture.get(l, timeUnit);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(ApiFutureToListenableFuture.class.getSimpleName())
.add("apiFuture", apiFuture)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/
package com.google.api.core;

import com.google.common.base.MoreObjects;
import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture;
import com.google.common.util.concurrent.ListenableFuture;

Expand All @@ -39,4 +40,10 @@ public class ListenableFutureToApiFuture<V> extends SimpleForwardingListenableFu
public ListenableFutureToApiFuture(ListenableFuture<V> delegate) {
super(delegate);
}

public String toString() {
return MoreObjects.toStringHelper(ListenableFutureToApiFuture.class)
.add("delegate", delegate())
.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2024, Google Inc.
*
* 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 Inc. 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.core;

import static com.google.common.truth.Truth.assertThat;

import org.junit.jupiter.api.Test;

class ApiFutureToListenableFutureTest {
@Test
void testThatInnerToStringIsNotLost() {
String customInnerToString = "my-custom-inner-tostring";
ApiFuture<String> apiFuture =
new AbstractApiFuture<String>() {
@Override
public String toString() {
return customInnerToString;
}
};
ApiFutureToListenableFuture<String> listenableFuture =
new ApiFutureToListenableFuture<>(apiFuture);
assertThat(listenableFuture.toString()).contains(customInnerToString);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
package com.google.api.core;

import com.google.common.truth.Truth;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import org.junit.jupiter.api.Test;

Expand All @@ -42,4 +44,20 @@ void testGet() throws Exception {
future.set(3);
Truth.assertThat(apiFuture.get()).isEqualTo(3);
}

@Test
void testToStringShowsUnderlyingFutureToString() {
String customInnerFutureDesc = "my-custom-toString-impl";
ListenableFuture<String> listenableFuture =
new AbstractFuture<String>() {
@Override
public String toString() {
return customInnerFutureDesc;
}
};

ListenableFutureToApiFuture<String> apiFuture =
new ListenableFutureToApiFuture<>(listenableFuture);
Truth.assertThat(apiFuture.toString()).contains(customInnerFutureDesc);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.gax.tracing.ApiTracer;
import com.google.common.base.MoreObjects;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.MoreExecutors;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -265,6 +266,16 @@ private void setAttemptResult(Throwable throwable, ResponseT response, boolean s
}
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this.getClass())
.add("super", pendingToString())
.add("latestCompletedAttemptResult", this.latestCompletedAttemptResult)
.add("attemptResult", this.attemptResult)
.add("attemptSettings", this.attemptSettings)
.toString();
}

private class CompletionListener implements Runnable {
@Override
public void run() {
Expand Down

0 comments on commit afecb8c

Please sign in to comment.