Skip to content

Commit

Permalink
test: removes flaky close result set test (#888)
Browse files Browse the repository at this point in the history
This test was introduced in order to debug stuck transactions. The cause was unrelated though.
The test here is flaky, because there is a race condition between setting the stream field in the AbstractResultSet and checking it in the close method. Since the result set is not intended to be thread safe, we will not be synchronizing this variable.
  • Loading branch information
thiagotnunes committed Feb 18, 2021
1 parent 864f2a2 commit b2b7bb2
Showing 1 changed file with 0 additions and 73 deletions.
Expand Up @@ -36,7 +36,6 @@
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
import com.google.cloud.spanner.TransactionRunner.TransactionCallable;
import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.protobuf.AbstractMessage;
Expand All @@ -61,15 +60,13 @@
import java.util.Collection;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.After;
Expand Down Expand Up @@ -1501,76 +1498,6 @@ public Void run(TransactionContext transaction) throws Exception {
assertThat(countRequests(CommitRequest.class)).isEqualTo(1);
}

@Test
public void testCloseResultSetWhileRequestInFlight() throws Exception {
DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
final ExecutorService service = Executors.newSingleThreadExecutor();
try {
client
.readWriteTransaction()
.run(
new TransactionCallable<Void>() {
@Override
public Void run(TransactionContext transaction) throws Exception {
final ResultSet rs = transaction.executeQuery(SELECT1);
// Prevent the server from executing the query.
final CountDownLatch latch = new CountDownLatch(1);
mockSpanner.freeze();
service.submit(
new Runnable() {
@Override
public void run() {
try {
// This call will be stuck on the server until the mock server is
// unfrozen.
rs.next();
} finally {
latch.countDown();
}
}
});

// First wait for the request to be on the server and then close the result set
// while the request is in flight.
mockSpanner.waitForRequestsToContain(
new Predicate<AbstractMessage>() {
@Override
public boolean apply(AbstractMessage input) {
return input instanceof ExecuteSqlRequest
&& ((ExecuteSqlRequest) input).getTransaction().hasBegin();
}
},
1000L);
rs.close();
// The next statement should now fail before it is sent to the server because
// the first statement failed to return a transaction while the result set was
// still open.
mockSpanner.unfreeze();
latch.await(1L, TimeUnit.SECONDS);
try {
transaction.executeUpdate(UPDATE_STATEMENT);
fail("missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.FAILED_PRECONDITION);
assertThat(e.getMessage())
.contains("ResultSet was closed before a transaction id was returned");
}
return null;
}
});
fail("missing expected exception");
} catch (SpannerException e) {
// The commit request will also fail, which means that the entire transaction will fail.
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.FAILED_PRECONDITION);
assertThat(e.getMessage())
.contains("ResultSet was closed before a transaction id was returned");
}
service.shutdown();
assertThat(countRequests(BeginTransactionRequest.class)).isEqualTo(0);
assertThat(countRequests(ExecuteSqlRequest.class)).isEqualTo(1);
assertThat(countRequests(CommitRequest.class)).isEqualTo(0);
}

@Test
public void testQueryWithInlineBeginDidNotReturnTransaction() {
DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
Expand Down

0 comments on commit b2b7bb2

Please sign in to comment.