Skip to content

Commit 741e40c

Browse files
diegomarquezpdiegomarquezp
authored
fix: improve information on CancellationException for LROs (#2236)
* fix: point to documentation on LRO CancelledException * mvn fmt:format * add test for http IT in LRO * format code * use final variable on LRO cancellation * Wrap guava's CancellationException in our own Exception * reset test and basic retry future * use logging to inform of failed LRO * add license and comment to fake log handler * format code * fix license * format souce * decrease LRO timeout logging level to WARNING * fix logging threshold check * use common log handler setup for ITLongRunningOperation * fix license header year * format source * reset lro it * change to unit tests * reset testjar exports * simplify unit test * use instance variables in test
1 parent 9d30ed9 commit 741e40c

File tree

5 files changed

+193
-22
lines changed

5 files changed

+193
-22
lines changed

‎gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java

+1-22
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import com.google.api.gax.rpc.StreamController;
4444
import com.google.api.gax.rpc.UnaryCallSettings;
4545
import com.google.api.gax.rpc.UnaryCallable;
46+
import com.google.api.gax.util.FakeLogHandler;
4647
import com.google.common.base.Preconditions;
4748
import com.google.common.collect.ImmutableList;
4849
import com.google.common.collect.Lists;
@@ -66,9 +67,6 @@
6667
import java.util.concurrent.ScheduledFuture;
6768
import java.util.concurrent.TimeUnit;
6869
import java.util.concurrent.atomic.AtomicInteger;
69-
import java.util.logging.Handler;
70-
import java.util.logging.LogRecord;
71-
import java.util.stream.Collectors;
7270
import org.junit.After;
7371
import org.junit.Assert;
7472
import org.junit.Test;
@@ -717,23 +715,4 @@ public void testDoubleRelease() throws Exception {
717715
ChannelPool.LOG.removeHandler(logHandler);
718716
}
719717
}
720-
721-
private static class FakeLogHandler extends Handler {
722-
List<LogRecord> records = new ArrayList<>();
723-
724-
@Override
725-
public void publish(LogRecord record) {
726-
records.add(record);
727-
}
728-
729-
@Override
730-
public void flush() {}
731-
732-
@Override
733-
public void close() throws SecurityException {}
734-
735-
List<String> getAllMessages() {
736-
return records.stream().map(LogRecord::getMessage).collect(Collectors.toList());
737-
}
738-
}
739718
}

‎gax-java/gax/pom.xml

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
<include>com/google/api/gax/core/**</include>
7777
<include>com/google/api/gax/rpc/testing/**</include>
7878
<include>com/google/api/gax/rpc/mtls/**</include>
79+
<include>com/google/api/gax/util/**</include>
7980
</includes>
8081
</configuration>
8182
</execution>

‎gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java

+18
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,25 @@
3535
import com.google.api.gax.retrying.ExponentialRetryAlgorithm;
3636
import com.google.api.gax.retrying.RetrySettings;
3737
import com.google.api.gax.retrying.TimedAttemptSettings;
38+
import com.google.common.annotations.VisibleForTesting;
3839
import java.util.concurrent.CancellationException;
40+
import java.util.logging.Level;
41+
import java.util.logging.Logger;
3942

4043
/**
4144
* Operation timed polling algorithm, which uses exponential backoff factor for determining when the
4245
* next polling operation should be executed. If the polling exceeds the total timeout this
4346
* algorithm cancels polling.
4447
*/
4548
public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm {
49+
50+
@VisibleForTesting
51+
static final Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName());
52+
53+
@VisibleForTesting
54+
static final String LRO_TROUBLESHOOTING_LINK =
55+
"https://github.com/googleapis/google-cloud-java#lro-timeouts";
56+
4657
/**
4758
* Creates the polling algorithm, using the default {@code NanoClock} for time computations.
4859
*
@@ -77,6 +88,13 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings)
7788
if (super.shouldRetry(nextAttemptSettings)) {
7889
return true;
7990
}
91+
if (LOGGER.isLoggable(Level.WARNING)) {
92+
LOGGER.log(
93+
Level.WARNING,
94+
"The task has been cancelled. Please refer to "
95+
+ LRO_TROUBLESHOOTING_LINK
96+
+ " for more information");
97+
}
8098
throw new CancellationException();
8199
}
82100

‎gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
* Copyright 2023 Google LLC
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
* * Neither the name of Google LLC nor the names of its
15+
* contributors may be used to endorse or promote products derived from
16+
* this software without specific prior written permission.
17+
*
18+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
19+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
20+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
21+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
22+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
23+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
24+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
25+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
26+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
27+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
28+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
29+
*/
30+
package com.google.api.gax.longrunning;
31+
32+
import static org.junit.Assert.assertThrows;
33+
import static org.junit.Assert.assertTrue;
34+
import static org.junit.Assert.fail;
35+
36+
import com.google.api.gax.core.FakeApiClock;
37+
import com.google.api.gax.retrying.RetrySettings;
38+
import com.google.api.gax.retrying.TimedAttemptSettings;
39+
import com.google.api.gax.util.FakeLogHandler;
40+
import java.util.concurrent.CancellationException;
41+
import org.junit.After;
42+
import org.junit.Before;
43+
import org.junit.Test;
44+
import org.threeten.bp.Duration;
45+
46+
public class OperationTimedPollAlgorithmTest {
47+
48+
private static final RetrySettings FAST_RETRY_SETTINGS =
49+
RetrySettings.newBuilder()
50+
.setInitialRetryDelay(Duration.ofMillis(1L))
51+
.setRetryDelayMultiplier(1)
52+
.setMaxRetryDelay(Duration.ofMillis(1L))
53+
.setInitialRpcTimeout(Duration.ofMillis(1L))
54+
.setMaxAttempts(0)
55+
.setJittered(false)
56+
.setRpcTimeoutMultiplier(1)
57+
.setMaxRpcTimeout(Duration.ofMillis(1L))
58+
.setTotalTimeout(Duration.ofMillis(5L))
59+
.build();
60+
private TimedAttemptSettings timedAttemptSettings;
61+
private FakeApiClock clock;
62+
63+
private FakeLogHandler logHandler;
64+
65+
@Before
66+
public void setUp() {
67+
logHandler = new FakeLogHandler();
68+
OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler);
69+
clock = new FakeApiClock(System.nanoTime());
70+
timedAttemptSettings =
71+
TimedAttemptSettings.newBuilder()
72+
.setGlobalSettings(FAST_RETRY_SETTINGS)
73+
.setRetryDelay(Duration.ofMillis(1l))
74+
.setRpcTimeout(Duration.ofMillis(1l))
75+
.setRandomizedRetryDelay(Duration.ofMillis(1l))
76+
.setAttemptCount(0)
77+
.setFirstAttemptStartTimeNanos(clock.nanoTime())
78+
.build();
79+
}
80+
81+
@After
82+
public void tearDown() {
83+
OperationTimedPollAlgorithm.LOGGER.removeHandler(logHandler);
84+
// redundant null assignment for readability - a new log handler will be used
85+
logHandler = null;
86+
}
87+
88+
@Test
89+
public void testAlgorithmThatShouldRetry_doesNotLogTimeoutHelpMessage() {
90+
OperationTimedPollAlgorithm algorithm =
91+
OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock);
92+
try {
93+
algorithm.shouldRetry(timedAttemptSettings);
94+
} catch (CancellationException ex) {
95+
fail("Unexpected unsuccessful shouldRetry()");
96+
}
97+
assertTrue(
98+
logHandler.getAllMessages().stream()
99+
.noneMatch(
100+
entry -> entry.contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK)));
101+
}
102+
103+
@Test
104+
public void testAlgorithmThatShouldNotRetry_logsTimeoutHelpMessage() {
105+
OperationTimedPollAlgorithm algorithm =
106+
OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock);
107+
clock.incrementNanoTime(1 * 1000 * 1000 * 1000); // force rpc timeout
108+
assertThrows(CancellationException.class, () -> algorithm.shouldRetry(timedAttemptSettings));
109+
assertTrue(
110+
logHandler.getAllMessages().stream()
111+
.anyMatch(
112+
entry -> entry.contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK)));
113+
}
114+
}

‎gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright 2023 Google LLC
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
* * Neither the name of Google LLC nor the names of its
15+
* contributors may be used to endorse or promote products derived from
16+
* this software without specific prior written permission.
17+
*
18+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
19+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
20+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
21+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
22+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
23+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
24+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
25+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
26+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
27+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
28+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
29+
*/
30+
package com.google.api.gax.util;
31+
32+
import java.util.ArrayList;
33+
import java.util.List;
34+
import java.util.logging.Handler;
35+
import java.util.logging.LogRecord;
36+
import java.util.stream.Collectors;
37+
38+
/*
39+
* Convenience class that stores the log entries produced by any logger
40+
* It can then be inspected - its entries are a list log records
41+
*/
42+
public class FakeLogHandler extends Handler {
43+
List<LogRecord> records = new ArrayList<>();
44+
45+
@Override
46+
public void publish(LogRecord record) {
47+
records.add(record);
48+
}
49+
50+
@Override
51+
public void flush() {}
52+
53+
@Override
54+
public void close() throws SecurityException {}
55+
56+
public List<String> getAllMessages() {
57+
return records.stream().map(LogRecord::getMessage).collect(Collectors.toList());
58+
}
59+
}

0 commit comments

Comments
 (0)