Skip to content

Commit d76d159

Browse files
test(spanner): unflake multiUseReadOnlyTransactionCanUseInlineBeginForReadAsync (#14329)
In `SessionImplTest#multiUseReadOnlyTransactionCanUseInlineBeginForReadAsync`, mock `rpc.read` invoked stream callbacks synchronously before returning `StreamingCall`. This caused a background thread to process rows and invoke `startStream` while `this.stream` was still unassigned, triggering a duplicate request (`expected:<1> but was:<2>`). Fix this deterministically by delaying `consumer.onPartialResultSet` and `consumer.onCompleted` until `StreamingCall.request(int)` is called. In gRPC streaming, `this.stream` is guaranteed to be assigned before `request()` is executed, eliminating the race condition without relying on background threads or executors.
1 parent beea42f commit d76d159

1 file changed

Lines changed: 34 additions & 13 deletions

File tree

‎java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import java.util.concurrent.Executors;
7272
import java.util.concurrent.Future;
7373
import java.util.concurrent.TimeUnit;
74+
import java.util.concurrent.atomic.AtomicBoolean;
7475
import java.util.concurrent.atomic.AtomicInteger;
7576
import javax.annotation.Nullable;
7677
import org.junit.Before;
@@ -817,21 +818,41 @@ public void multiUseReadOnlyTransactionInlineBeginConcurrentStartWaitsForTransac
817818
@Test
818819
public void multiUseReadOnlyTransactionCanUseInlineBeginForReadAsync() throws Exception {
819820
PartialResultSet resultSet = inlineBeginResultSet("async-inline-tx");
820-
final ArgumentCaptor<SpannerRpc.ResultStreamConsumer> consumer =
821+
final ArgumentCaptor<SpannerRpc.ResultStreamConsumer> consumerCaptor =
821822
ArgumentCaptor.forClass(SpannerRpc.ResultStreamConsumer.class);
822-
final ArgumentCaptor<ReadRequest> request = ArgumentCaptor.forClass(ReadRequest.class);
823-
Mockito.when(rpc.read(request.capture(), consumer.capture(), anyMap(), any(), eq(false)))
823+
final ArgumentCaptor<ReadRequest> requestCaptor = ArgumentCaptor.forClass(ReadRequest.class);
824+
825+
Mockito.when(
826+
rpc.read(requestCaptor.capture(), consumerCaptor.capture(), anyMap(), any(), eq(false)))
824827
.then(
825828
invocation -> {
826-
consumer.getValue().onPartialResultSet(resultSet);
827-
consumer.getValue().onCompleted();
828-
return new NoOpStreamingCall();
829+
SpannerRpc.ResultStreamConsumer consumer = invocation.getArgument(1);
830+
return new SpannerRpc.StreamingCall() {
831+
private final AtomicBoolean requested = new AtomicBoolean();
832+
833+
@Override
834+
public ApiCallContext getCallContext() {
835+
return GrpcCallContext.createDefault();
836+
}
837+
838+
@Override
839+
public void request(int numMessages) {
840+
if (requested.compareAndSet(false, true)) {
841+
consumer.onPartialResultSet(resultSet);
842+
consumer.onCompleted();
843+
}
844+
}
845+
846+
@Override
847+
public void cancel(@Nullable String message) {}
848+
};
829849
});
830850

831-
try (ReadOnlyTransaction txn = inlineReadOnlyTransaction()) {
832-
try (AsyncResultSet rs =
833-
txn.readAsync("Dummy", KeySet.all(), Collections.singletonList("C"))) {
834-
rs.setCallback(
851+
try (ReadOnlyTransaction transaction = inlineReadOnlyTransaction()) {
852+
try (AsyncResultSet resultSetAsync =
853+
transaction.readAsync("Dummy", KeySet.all(), Collections.singletonList("C"))) {
854+
resultSetAsync
855+
.setCallback(
835856
Runnable::run,
836857
asyncResultSet -> {
837858
while (asyncResultSet.tryNext() == AsyncResultSet.CursorState.OK) {}
@@ -841,12 +862,12 @@ public void multiUseReadOnlyTransactionCanUseInlineBeginForReadAsync() throws Ex
841862
}
842863
assertEquals(
843864
Timestamp.fromProto(Timestamps.parse("2015-10-01T10:54:20.021Z")),
844-
txn.getReadTimestamp());
865+
transaction.getReadTimestamp());
845866
}
846867

847868
Mockito.verify(rpc, Mockito.never()).beginTransaction(Mockito.any(), anyMap(), eq(false));
848-
assertEquals(1, request.getAllValues().size());
849-
assertThat(request.getValue().getTransaction().hasBegin()).isTrue();
869+
assertEquals(1, requestCaptor.getAllValues().size());
870+
assertThat(requestCaptor.getValue().getTransaction().hasBegin()).isTrue();
850871
}
851872

852873
@Test

0 commit comments

Comments
 (0)