Skip to content

Commit

Permalink
fix: prevent illegal negative timeout values into thread sleep() meth…
Browse files Browse the repository at this point in the history
…od 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.
  • Loading branch information
arpan14 committed Oct 31, 2023
1 parent 2b17f09 commit 1c26cf6
Showing 1 changed file with 20 additions and 5 deletions.
Expand Up @@ -134,7 +134,10 @@ public void simpleInsert() throws InterruptedException {
assertThat(row.getBoolean(1)).isTrue();
break;
} catch (AbortedException e) {
Thread.sleep(e.getRetryDelayInMillis());
long retryDelayInMillis = e.getRetryDelayInMillis();
if (retryDelayInMillis > 0) {
Thread.sleep(retryDelayInMillis);
}
txn = manager.resetForRetry();
}
}
Expand All @@ -158,7 +161,10 @@ public void invalidInsert() throws InterruptedException {
manager.commit();
fail("Expected exception");
} catch (AbortedException e) {
Thread.sleep(e.getRetryDelayInMillis());
long retryDelayInMillis = e.getRetryDelayInMillis();
if (retryDelayInMillis > 0) {
Thread.sleep(retryDelayInMillis);
}
txn = manager.resetForRetry();
} catch (SpannerException e) {
// expected
Expand Down Expand Up @@ -188,7 +194,10 @@ public void rollback() throws InterruptedException {
manager.rollback();
break;
} catch (AbortedException e) {
Thread.sleep(e.getRetryDelayInMillis());
long retryDelayInMillis = e.getRetryDelayInMillis();
if (retryDelayInMillis > 0) {
Thread.sleep(retryDelayInMillis);
}
txn = manager.resetForRetry();
}
}
Expand Down Expand Up @@ -231,7 +240,10 @@ public void abortAndRetry() throws InterruptedException {
manager1.commit();
break;
} catch (AbortedException e) {
Thread.sleep(e.getRetryDelayInMillis());
long retryDelayInMillis = e.getRetryDelayInMillis();
if (retryDelayInMillis > 0) {
Thread.sleep(retryDelayInMillis);
}
// It is possible that it was txn2 that aborted.
// In that case we should just retry without resetting anything.
if (manager1.getState() == TransactionState.ABORTED) {
Expand Down Expand Up @@ -278,7 +290,10 @@ public void testTransactionManagerReturnsCommitStats() throws InterruptedExcepti
assertEquals(2L, manager.getCommitResponse().getCommitStats().getMutationCount());
break;
} catch (AbortedException e) {
Thread.sleep(e.getRetryDelayInMillis());
long retryDelayInMillis = e.getRetryDelayInMillis();
if (retryDelayInMillis > 0) {
Thread.sleep(retryDelayInMillis);
}
transaction = manager.resetForRetry();
}
}
Expand Down

0 comments on commit 1c26cf6

Please sign in to comment.