Skip to content

Commit 1c26cf6

Browse files
authored
fix: prevent illegal negative timeout values into thread sleep() method in ITTransactionManagerTest. (#2715)
* Issue - https://togithub.com/googleapis/java-spanner/issues/2634 * Root Cause - In cases where we are receiving AbortedException, the retry delay is set as -1 for the exception. Negative value is not an acceptable input to Thread.sleep() method and hence we are getting IllegalArgumentException. This is resulting in flaky unit test behaviour.
1 parent 2b17f09 commit 1c26cf6

File tree

1 file changed

+20
-5
lines changed

1 file changed

+20
-5
lines changed

google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITTransactionManagerTest.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ public void simpleInsert() throws InterruptedException {
134134
assertThat(row.getBoolean(1)).isTrue();
135135
break;
136136
} catch (AbortedException e) {
137-
Thread.sleep(e.getRetryDelayInMillis());
137+
long retryDelayInMillis = e.getRetryDelayInMillis();
138+
if (retryDelayInMillis > 0) {
139+
Thread.sleep(retryDelayInMillis);
140+
}
138141
txn = manager.resetForRetry();
139142
}
140143
}
@@ -158,7 +161,10 @@ public void invalidInsert() throws InterruptedException {
158161
manager.commit();
159162
fail("Expected exception");
160163
} catch (AbortedException e) {
161-
Thread.sleep(e.getRetryDelayInMillis());
164+
long retryDelayInMillis = e.getRetryDelayInMillis();
165+
if (retryDelayInMillis > 0) {
166+
Thread.sleep(retryDelayInMillis);
167+
}
162168
txn = manager.resetForRetry();
163169
} catch (SpannerException e) {
164170
// expected
@@ -188,7 +194,10 @@ public void rollback() throws InterruptedException {
188194
manager.rollback();
189195
break;
190196
} catch (AbortedException e) {
191-
Thread.sleep(e.getRetryDelayInMillis());
197+
long retryDelayInMillis = e.getRetryDelayInMillis();
198+
if (retryDelayInMillis > 0) {
199+
Thread.sleep(retryDelayInMillis);
200+
}
192201
txn = manager.resetForRetry();
193202
}
194203
}
@@ -231,7 +240,10 @@ public void abortAndRetry() throws InterruptedException {
231240
manager1.commit();
232241
break;
233242
} catch (AbortedException e) {
234-
Thread.sleep(e.getRetryDelayInMillis());
243+
long retryDelayInMillis = e.getRetryDelayInMillis();
244+
if (retryDelayInMillis > 0) {
245+
Thread.sleep(retryDelayInMillis);
246+
}
235247
// It is possible that it was txn2 that aborted.
236248
// In that case we should just retry without resetting anything.
237249
if (manager1.getState() == TransactionState.ABORTED) {
@@ -278,7 +290,10 @@ public void testTransactionManagerReturnsCommitStats() throws InterruptedExcepti
278290
assertEquals(2L, manager.getCommitResponse().getCommitStats().getMutationCount());
279291
break;
280292
} catch (AbortedException e) {
281-
Thread.sleep(e.getRetryDelayInMillis());
293+
long retryDelayInMillis = e.getRetryDelayInMillis();
294+
if (retryDelayInMillis > 0) {
295+
Thread.sleep(retryDelayInMillis);
296+
}
282297
transaction = manager.resetForRetry();
283298
}
284299
}

0 commit comments

Comments
 (0)