Skip to content

Commit dd4c944

Browse files
harshachintaharshachinta
committed
chore(spanner): uncomment TODO changes
1 parent 303c9d9 commit dd4c944

File tree

2 files changed

+90
-93
lines changed

2 files changed

+90
-93
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ static TransactionOptions createReadWriteTransactionOptions(
8181
}
8282
if (previousTransactionId != null
8383
&& previousTransactionId != com.google.protobuf.ByteString.EMPTY) {
84-
// TODO(sriharshach): uncomment this when multiplexed session R/W proto is published
85-
// readWrite.setMultiplexedSessionPreviousTransactionId(previousTransactionId);
84+
readWrite.setMultiplexedSessionPreviousTransactionId(previousTransactionId);
8685
}
8786
transactionOptions.setReadWrite(readWrite);
8887
return transactionOptions.build();

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

Lines changed: 89 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@
3232
import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
3333
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
3434
import com.google.cloud.spanner.Options.RpcPriority;
35+
import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl;
3536
import com.google.cloud.spanner.connection.RandomResultSetGenerator;
3637
import com.google.common.base.Stopwatch;
3738
import com.google.common.collect.ImmutableList;
3839
import com.google.protobuf.ByteString;
40+
import com.google.spanner.v1.BeginTransactionRequest;
3941
import com.google.spanner.v1.CommitRequest;
4042
import com.google.spanner.v1.ExecuteSqlRequest;
4143
import com.google.spanner.v1.RequestOptions.Priority;
@@ -46,6 +48,7 @@
4648
import java.util.List;
4749
import java.util.Set;
4850
import java.util.UUID;
51+
import java.util.concurrent.atomic.AtomicReference;
4952
import java.util.stream.Collectors;
5053
import org.junit.Before;
5154
import org.junit.BeforeClass;
@@ -476,99 +479,94 @@ public void testWriteAtLeastOnceWithExcludeTxnFromChangeStreams() {
476479
assertEquals(1L, client.multiplexedSessionDatabaseClient.getNumSessionsReleased().get());
477480
}
478481

479-
// TODO(sriharshach): Uncomment test once Lock order preservation proto is published
480-
/*
481-
@Test
482-
public void testAbortedReadWriteTxnUsesPreviousTxnIdOnRetryWithInlineBegin()
483-
throws InterruptedException {
484-
DatabaseClientImpl client =
485-
(DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
486-
// Force the Commit RPC to return Aborted the first time it is called. The exception is cleared
487-
// after the first call, so the retry should succeed.
488-
mockSpanner.setCommitExecutionTime(
489-
SimulatedExecutionTime.ofException(
490-
mockSpanner.createAbortedException(ByteString.copyFromUtf8("test"))));
491-
Thread.sleep(10000);
492-
TransactionRunner runner = client.readWriteTransaction();
493-
AtomicReference<ByteString> validTransactionId = new AtomicReference<>();
494-
runner.run(
495-
transaction -> {
496-
try (ResultSet resultSet = transaction.executeQuery(STATEMENT)) {
497-
while (resultSet.next()) {}
498-
}
499-
500-
TransactionContextImpl impl = (TransactionContextImpl) transaction;
501-
if (validTransactionId.get() == null) {
502-
// Track the first not-null transactionId. This transaction gets ABORTED during commit
503-
// operation and gets retried.
504-
validTransactionId.set(impl.transactionId);
505-
}
506-
return null;
507-
});
508-
509-
List<ExecuteSqlRequest> executeSqlRequests =
510-
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
511-
assertEquals(2, executeSqlRequests.size());
512-
513-
// Verify the requests are executed using multiplexed sessions
514-
for (ExecuteSqlRequest request : executeSqlRequests) {
515-
assertTrue(mockSpanner.getSession(request.getSession()).getMultiplexed());
516-
}
482+
@Test
483+
public void testAbortedReadWriteTxnUsesPreviousTxnIdOnRetryWithInlineBegin()
484+
throws InterruptedException {
485+
DatabaseClientImpl client =
486+
(DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
487+
// Force the Commit RPC to return Aborted the first time it is called. The exception is cleared
488+
// after the first call, so the retry should succeed.
489+
mockSpanner.setCommitExecutionTime(
490+
SimulatedExecutionTime.ofException(
491+
mockSpanner.createAbortedException(ByteString.copyFromUtf8("test"))));
492+
Thread.sleep(10000);
493+
TransactionRunner runner = client.readWriteTransaction();
494+
AtomicReference<ByteString> validTransactionId = new AtomicReference<>();
495+
runner.run(
496+
transaction -> {
497+
try (ResultSet resultSet = transaction.executeQuery(STATEMENT)) {
498+
while (resultSet.next()) {}
499+
}
500+
501+
TransactionContextImpl impl = (TransactionContextImpl) transaction;
502+
if (validTransactionId.get() == null) {
503+
// Track the first not-null transactionId. This transaction gets ABORTED during commit
504+
// operation and gets retried.
505+
validTransactionId.set(impl.transactionId);
506+
}
507+
return null;
508+
});
509+
510+
List<ExecuteSqlRequest> executeSqlRequests =
511+
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
512+
assertEquals(2, executeSqlRequests.size());
517513

518-
// Verify that the first request uses inline begin, and the previous transaction ID is set to
519-
// ByteString.EMPTY
520-
assertTrue(executeSqlRequests.get(0).hasTransaction());
521-
assertTrue(executeSqlRequests.get(0).getTransaction().hasBegin());
522-
assertTrue(executeSqlRequests.get(0).getTransaction().getBegin().hasReadWrite());
523-
assertNotNull(
524-
executeSqlRequests
525-
.get(0)
526-
.getTransaction()
527-
.getBegin()
528-
.getReadWrite()
529-
.getMultiplexedSessionPreviousTransactionId());
530-
assertEquals(
531-
ByteString.EMPTY,
532-
executeSqlRequests
533-
.get(0)
534-
.getTransaction()
535-
.getBegin()
536-
.getReadWrite()
537-
.getMultiplexedSessionPreviousTransactionId());
538-
539-
// Verify that the second request uses inline begin, and the previous transaction ID is set
540-
// appropriately
541-
assertTrue(executeSqlRequests.get(1).hasTransaction());
542-
assertTrue(executeSqlRequests.get(1).getTransaction().hasBegin());
543-
assertTrue(executeSqlRequests.get(1).getTransaction().getBegin().hasReadWrite());
544-
assertNotNull(
545-
executeSqlRequests
546-
.get(1)
547-
.getTransaction()
548-
.getBegin()
549-
.getReadWrite()
550-
.getMultiplexedSessionPreviousTransactionId());
551-
assertNotEquals(
552-
ByteString.EMPTY,
553-
executeSqlRequests
554-
.get(1)
555-
.getTransaction()
556-
.getBegin()
557-
.getReadWrite()
558-
.getMultiplexedSessionPreviousTransactionId());
559-
assertEquals(
560-
validTransactionId.get(),
561-
executeSqlRequests
562-
.get(1)
563-
.getTransaction()
564-
.getBegin()
565-
.getReadWrite()
566-
.getMultiplexedSessionPreviousTransactionId());
514+
// Verify the requests are executed using multiplexed sessions
515+
for (ExecuteSqlRequest request : executeSqlRequests) {
516+
assertTrue(mockSpanner.getSession(request.getSession()).getMultiplexed());
567517
}
568-
*/
569518

570-
// TODO(sriharshach): Uncomment test once Lock order preservation proto is published
571-
/*
519+
// Verify that the first request uses inline begin, and the previous transaction ID is set to
520+
// ByteString.EMPTY
521+
assertTrue(executeSqlRequests.get(0).hasTransaction());
522+
assertTrue(executeSqlRequests.get(0).getTransaction().hasBegin());
523+
assertTrue(executeSqlRequests.get(0).getTransaction().getBegin().hasReadWrite());
524+
assertNotNull(
525+
executeSqlRequests
526+
.get(0)
527+
.getTransaction()
528+
.getBegin()
529+
.getReadWrite()
530+
.getMultiplexedSessionPreviousTransactionId());
531+
assertEquals(
532+
ByteString.EMPTY,
533+
executeSqlRequests
534+
.get(0)
535+
.getTransaction()
536+
.getBegin()
537+
.getReadWrite()
538+
.getMultiplexedSessionPreviousTransactionId());
539+
540+
// Verify that the second request uses inline begin, and the previous transaction ID is set
541+
// appropriately
542+
assertTrue(executeSqlRequests.get(1).hasTransaction());
543+
assertTrue(executeSqlRequests.get(1).getTransaction().hasBegin());
544+
assertTrue(executeSqlRequests.get(1).getTransaction().getBegin().hasReadWrite());
545+
assertNotNull(
546+
executeSqlRequests
547+
.get(1)
548+
.getTransaction()
549+
.getBegin()
550+
.getReadWrite()
551+
.getMultiplexedSessionPreviousTransactionId());
552+
assertNotEquals(
553+
ByteString.EMPTY,
554+
executeSqlRequests
555+
.get(1)
556+
.getTransaction()
557+
.getBegin()
558+
.getReadWrite()
559+
.getMultiplexedSessionPreviousTransactionId());
560+
assertEquals(
561+
validTransactionId.get(),
562+
executeSqlRequests
563+
.get(1)
564+
.getTransaction()
565+
.getBegin()
566+
.getReadWrite()
567+
.getMultiplexedSessionPreviousTransactionId());
568+
}
569+
572570
@Test
573571
public void testAbortedReadWriteTxnUsesPreviousTxnIdOnRetryWithExplicitBegin()
574572
throws InterruptedException {
@@ -656,7 +654,7 @@ public void testAbortedReadWriteTxnUsesPreviousTxnIdOnRetryWithExplicitBegin()
656654
.getOptions()
657655
.getReadWrite()
658656
.getMultiplexedSessionPreviousTransactionId());
659-
}*/
657+
}
660658

661659
private void waitForSessionToBeReplaced(DatabaseClientImpl client) {
662660
assertNotNull(client.multiplexedSessionDatabaseClient);

0 commit comments

Comments
 (0)