Skip to content

Commit db2e31f

Browse files
harshachintaharshachinta
committed
chore(spanner): review comments
1 parent 7ddcc4c commit db2e31f

File tree

5 files changed

+48
-44
lines changed

5 files changed

+48
-44
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class DatabaseClientImpl implements DatabaseClient {
4848
/* useMultiplexedSessionBlindWrite = */ false,
4949
/* multiplexedSessionDatabaseClient = */ null,
5050
tracer,
51-
false);
51+
/* useMultiplexedSessionForRW = */ false);
5252
}
5353

5454
@VisibleForTesting

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public ReadContext singleUse() {
5858
this.sessionFuture,
5959
sessionReference ->
6060
new MultiplexedSessionTransaction(
61-
client, span, sessionReference, NO_CHANNEL_HINT, true)
61+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ true)
6262
.singleUse(),
6363
MoreExecutors.directExecutor()));
6464
}
@@ -70,7 +70,7 @@ public ReadContext singleUse(TimestampBound bound) {
7070
this.sessionFuture,
7171
sessionReference ->
7272
new MultiplexedSessionTransaction(
73-
client, span, sessionReference, NO_CHANNEL_HINT, true)
73+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ true)
7474
.singleUse(bound),
7575
MoreExecutors.directExecutor()));
7676
}
@@ -82,7 +82,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() {
8282
this.sessionFuture,
8383
sessionReference ->
8484
new MultiplexedSessionTransaction(
85-
client, span, sessionReference, NO_CHANNEL_HINT, true)
85+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ true)
8686
.singleUseReadOnlyTransaction(),
8787
MoreExecutors.directExecutor()));
8888
}
@@ -94,7 +94,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) {
9494
this.sessionFuture,
9595
sessionReference ->
9696
new MultiplexedSessionTransaction(
97-
client, span, sessionReference, NO_CHANNEL_HINT, true)
97+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ true)
9898
.singleUseReadOnlyTransaction(bound),
9999
MoreExecutors.directExecutor()));
100100
}
@@ -106,7 +106,7 @@ public ReadOnlyTransaction readOnlyTransaction() {
106106
this.sessionFuture,
107107
sessionReference ->
108108
new MultiplexedSessionTransaction(
109-
client, span, sessionReference, NO_CHANNEL_HINT, false)
109+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
110110
.readOnlyTransaction(),
111111
MoreExecutors.directExecutor()));
112112
}
@@ -118,7 +118,7 @@ public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) {
118118
this.sessionFuture,
119119
sessionReference ->
120120
new MultiplexedSessionTransaction(
121-
client, span, sessionReference, NO_CHANNEL_HINT, false)
121+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
122122
.readOnlyTransaction(bound),
123123
MoreExecutors.directExecutor()));
124124
}
@@ -132,34 +132,33 @@ public CommitResponse writeAtLeastOnceWithOptions(
132132
Iterable<Mutation> mutations, TransactionOption... options) throws SpannerException {
133133
SessionReference sessionReference = getSessionReference();
134134
try (MultiplexedSessionTransaction transaction =
135-
new MultiplexedSessionTransaction(client, span, sessionReference, NO_CHANNEL_HINT, true)) {
135+
new MultiplexedSessionTransaction(
136+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ true)) {
136137
return transaction.writeAtLeastOnceWithOptions(mutations, options);
137138
}
138139
}
139140

140-
/**
141-
* This is a blocking method, as the interface that it implements is also defined as a blocking
142-
* method.
143-
*/
141+
// This is a blocking method, as the interface that it implements is also defined as a blocking
142+
// method.
144143
@Override
145144
public Timestamp write(Iterable<Mutation> mutations) throws SpannerException {
146145
SessionReference sessionReference = getSessionReference();
147146
try (MultiplexedSessionTransaction transaction =
148-
new MultiplexedSessionTransaction(client, span, sessionReference, NO_CHANNEL_HINT, false)) {
147+
new MultiplexedSessionTransaction(
148+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)) {
149149
return transaction.write(mutations);
150150
}
151151
}
152152

153-
/**
154-
* This is a blocking method, as the interface that it implements is also defined as a blocking
155-
* method.
156-
*/
153+
// This is a blocking method, as the interface that it implements is also defined as a blocking
154+
// method.
157155
@Override
158156
public CommitResponse writeWithOptions(Iterable<Mutation> mutations, TransactionOption... options)
159157
throws SpannerException {
160158
SessionReference sessionReference = getSessionReference();
161159
try (MultiplexedSessionTransaction transaction =
162-
new MultiplexedSessionTransaction(client, span, sessionReference, NO_CHANNEL_HINT, false)) {
160+
new MultiplexedSessionTransaction(
161+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)) {
163162
return transaction.writeWithOptions(mutations, options);
164163
}
165164
}
@@ -171,7 +170,7 @@ public TransactionRunner readWriteTransaction(TransactionOption... options) {
171170
this.sessionFuture,
172171
sessionReference ->
173172
new MultiplexedSessionTransaction(
174-
client, span, sessionReference, NO_CHANNEL_HINT, false)
173+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
175174
.readWriteTransaction(options),
176175
MoreExecutors.directExecutor()));
177176
}
@@ -183,7 +182,7 @@ public TransactionManager transactionManager(TransactionOption... options) {
183182
this.sessionFuture,
184183
sessionReference ->
185184
new MultiplexedSessionTransaction(
186-
client, span, sessionReference, NO_CHANNEL_HINT, false)
185+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
187186
.transactionManager(options),
188187
MoreExecutors.directExecutor()));
189188
}
@@ -195,7 +194,7 @@ public AsyncRunner runAsync(TransactionOption... options) {
195194
this.sessionFuture,
196195
sessionReference ->
197196
new MultiplexedSessionTransaction(
198-
client, span, sessionReference, NO_CHANNEL_HINT, false)
197+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
199198
.runAsync(options),
200199
MoreExecutors.directExecutor()));
201200
}
@@ -207,7 +206,7 @@ public AsyncTransactionManager transactionManagerAsync(TransactionOption... opti
207206
this.sessionFuture,
208207
sessionReference ->
209208
new MultiplexedSessionTransaction(
210-
client, span, sessionReference, NO_CHANNEL_HINT, false)
209+
client, span, sessionReference, NO_CHANNEL_HINT, /* singleUse = */ false)
211210
.transactionManagerAsync(options),
212211
MoreExecutors.directExecutor()));
213212
}

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -370,71 +370,76 @@ private int getSingleUseChannelHint() {
370370

371371
@Override
372372
public Timestamp write(Iterable<Mutation> mutations) throws SpannerException {
373-
return createMultiplexedSessionTransaction(false).write(mutations);
373+
return createMultiplexedSessionTransaction(/* singleUse = */ false).write(mutations);
374374
}
375375

376376
@Override
377377
public CommitResponse writeWithOptions(
378378
final Iterable<Mutation> mutations, final TransactionOption... options)
379379
throws SpannerException {
380-
return createMultiplexedSessionTransaction(false).writeWithOptions(mutations, options);
380+
return createMultiplexedSessionTransaction(/* singleUse = */ false)
381+
.writeWithOptions(mutations, options);
381382
}
382383

383384
@Override
384385
public CommitResponse writeAtLeastOnceWithOptions(
385386
Iterable<Mutation> mutations, TransactionOption... options) throws SpannerException {
386-
return createMultiplexedSessionTransaction(true)
387+
return createMultiplexedSessionTransaction(/* singleUse = */ true)
387388
.writeAtLeastOnceWithOptions(mutations, options);
388389
}
389390

390391
@Override
391392
public ReadContext singleUse() {
392-
return createMultiplexedSessionTransaction(true).singleUse();
393+
return createMultiplexedSessionTransaction(/* singleUse = */ true).singleUse();
393394
}
394395

395396
@Override
396397
public ReadContext singleUse(TimestampBound bound) {
397-
return createMultiplexedSessionTransaction(true).singleUse(bound);
398+
return createMultiplexedSessionTransaction(/* singleUse = */ true).singleUse(bound);
398399
}
399400

400401
@Override
401402
public ReadOnlyTransaction singleUseReadOnlyTransaction() {
402-
return createMultiplexedSessionTransaction(true).singleUseReadOnlyTransaction();
403+
return createMultiplexedSessionTransaction(/* singleUse = */ true)
404+
.singleUseReadOnlyTransaction();
403405
}
404406

405407
@Override
406408
public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) {
407-
return createMultiplexedSessionTransaction(true).singleUseReadOnlyTransaction(bound);
409+
return createMultiplexedSessionTransaction(/* singleUse = */ true)
410+
.singleUseReadOnlyTransaction(bound);
408411
}
409412

410413
@Override
411414
public ReadOnlyTransaction readOnlyTransaction() {
412-
return createMultiplexedSessionTransaction(false).readOnlyTransaction();
415+
return createMultiplexedSessionTransaction(/* singleUse = */ false).readOnlyTransaction();
413416
}
414417

415418
@Override
416419
public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) {
417-
return createMultiplexedSessionTransaction(false).readOnlyTransaction(bound);
420+
return createMultiplexedSessionTransaction(/* singleUse = */ false).readOnlyTransaction(bound);
418421
}
419422

420423
@Override
421424
public TransactionRunner readWriteTransaction(TransactionOption... options) {
422-
return createMultiplexedSessionTransaction(false).readWriteTransaction(options);
425+
return createMultiplexedSessionTransaction(/* singleUse = */ false)
426+
.readWriteTransaction(options);
423427
}
424428

425429
@Override
426430
public TransactionManager transactionManager(TransactionOption... options) {
427-
return createMultiplexedSessionTransaction(false).transactionManager(options);
431+
return createMultiplexedSessionTransaction(/* singleUse = */ false).transactionManager(options);
428432
}
429433

430434
@Override
431435
public AsyncRunner runAsync(TransactionOption... options) {
432-
return createMultiplexedSessionTransaction(false).runAsync(options);
436+
return createMultiplexedSessionTransaction(/* singleUse = */ false).runAsync(options);
433437
}
434438

435439
@Override
436440
public AsyncTransactionManager transactionManagerAsync(TransactionOption... options) {
437-
return createMultiplexedSessionTransaction(false).transactionManagerAsync(options);
441+
return createMultiplexedSessionTransaction(/* singleUse = */ false)
442+
.transactionManagerAsync(options);
438443
}
439444

440445
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ DatabaseClientImpl createDatabaseClient(
5151
SessionPool pool,
5252
boolean useMultiplexedSessionBlindWriteIgnore,
5353
MultiplexedSessionDatabaseClient ignore,
54-
boolean useMultiplexedSessionForRW) {
54+
boolean useMultiplexedSessionForRWIgnore) {
5555
return new DatabaseClientWithClosedSessionImpl(clientId, pool, tracer);
5656
}
5757
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,10 @@ public void testMutationUsingWriteWithOptions() {
609609
assertNotNull(response.getCommitTimestamp());
610610

611611
List<CommitRequest> commitRequests = mockSpanner.getRequestsOfType(CommitRequest.class);
612-
assertThat(commitRequests).hasSize(1);
612+
assertEquals(1L, commitRequests.size());
613613
CommitRequest commit = commitRequests.get(0);
614614
assertNotNull(commit.getRequestOptions());
615-
assertThat(commit.getRequestOptions().getTransactionTag()).isEqualTo("app=spanner,env=test");
615+
assertEquals("app=spanner,env=test", commit.getRequestOptions().getTransactionTag());
616616
assertTrue(mockSpanner.getSession(commit.getSession()).getMultiplexed());
617617

618618
assertNotNull(client.multiplexedSessionDatabaseClient);
@@ -650,9 +650,9 @@ public void testReadWriteTransactionUsingAsyncTransactionManager() throws Except
650650
MoreExecutors.directExecutor());
651651
abortedLatch.await(10L, TimeUnit.SECONDS);
652652
CommitTimestampFuture commitTimestamp = updateCount.commitAsync();
653-
assertThat(updateCount.get()).isEqualTo(UPDATE_COUNT);
654-
assertThat(commitTimestamp.get()).isNotNull();
655-
assertThat(attempt.get()).isEqualTo(2);
653+
assertEquals(UPDATE_COUNT, updateCount.get().longValue());
654+
assertNotNull(commitTimestamp.get());
655+
assertEquals(2L, attempt.get());
656656
break;
657657
} catch (AbortedException e) {
658658
transactionContextFuture = manager.resetForRetryAsync();
@@ -695,12 +695,12 @@ public void testReadWriteTransactionUsingAsyncRunner() throws Exception {
695695
return updateCount1;
696696
},
697697
MoreExecutors.directExecutor());
698-
assertThat(updateCount.get()).isEqualTo(UPDATE_COUNT);
699-
assertThat(attempt.get()).isEqualTo(2);
698+
assertEquals(UPDATE_COUNT, updateCount.get().longValue());
699+
assertEquals(2L, attempt.get());
700700

701701
List<ExecuteSqlRequest> executeSqlRequests =
702702
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
703-
assertEquals(2, executeSqlRequests.size());
703+
assertEquals(2L, executeSqlRequests.size());
704704
assertEquals(executeSqlRequests.get(0).getSession(), executeSqlRequests.get(1).getSession());
705705

706706
// Verify the requests are executed using multiplexed sessions

0 commit comments

Comments
 (0)