Skip to content

Commit 19db330

Browse files
committed
fix lint
1 parent 83b84db commit 19db330

7 files changed

Lines changed: 66 additions & 37 deletions

File tree

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,8 @@ private boolean shouldUseInlinedBegin() {
423423
void beforeReadOrQuery() {
424424
super.beforeReadOrQuery();
425425
if (shouldUseInlinedBegin()) {
426+
// Keep the same nested transaction guard as the explicit BeginTransaction path. This checks
427+
// TransactionRunner's thread-local pending state, not the session's active transaction.
426428
SessionImpl.throwIfTransactionsPending();
427429
} else {
428430
initTransaction();
@@ -434,7 +436,8 @@ void beforeReadOrQuery() {
434436
TransactionSelector getTransactionSelector() {
435437
if (!shouldUseInlinedBegin()) {
436438
// No need for synchronization: super.readInternal() is always preceded by a check of
437-
// "transactionId" that provides a happens-before from initialization, and the value is never
439+
// "transactionId" that provides a happens-before from initialization, and the value is
440+
// never
438441
// changed afterwards.
439442
@SuppressWarnings("GuardedByChecker")
440443
TransactionSelector selector =
@@ -461,19 +464,18 @@ TransactionSelector getTransactionSelector() {
461464

462465
try {
463466
return TransactionSelector.newBuilder()
464-
.setId(
465-
futureToWaitFor.get(
466-
WAIT_FOR_INLINE_BEGIN_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS))
467+
.setId(futureToWaitFor.get(WAIT_FOR_INLINE_BEGIN_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS))
467468
.build();
468469
} catch (ExecutionException e) {
469470
throw SpannerExceptionFactory.asSpannerException(e.getCause());
470471
} catch (TimeoutException e) {
471472
throw SpannerExceptionFactory.newSpannerException(
472-
ErrorCode.ABORTED,
473+
ErrorCode.DEADLINE_EXCEEDED,
473474
"Timeout while waiting for an inlined read-only transaction to be returned by another"
474475
+ " statement.",
475476
e);
476477
} catch (InterruptedException e) {
478+
Thread.currentThread().interrupt();
477479
throw SpannerExceptionFactory.newSpannerExceptionForCancellation(null, e);
478480
}
479481
}
@@ -622,6 +624,13 @@ public void onDone(boolean withBeginTransaction) {
622624
super.onDone(withBeginTransaction);
623625
}
624626

627+
@Override
628+
void onStartFailed(boolean withBeginTransaction, Throwable t) {
629+
if (withBeginTransaction) {
630+
failTransactionIdFuture(t);
631+
}
632+
}
633+
625634
private void failTransactionIdFuture(Throwable t) {
626635
txnLock.lock();
627636
try {
@@ -1108,15 +1117,22 @@ CloseableIterator<PartialResultSet> startStream(
11081117
if (selector != null) {
11091118
request.setTransaction(selector);
11101119
}
1111-
SpannerRpc.StreamingCall call =
1112-
rpc.executeQuery(
1113-
request.build(),
1114-
stream.consumer(),
1115-
getTransactionChannelHint(),
1116-
requestId,
1117-
isRouteToLeader());
1120+
boolean withBeginTransaction = request.getTransaction().hasBegin();
1121+
SpannerRpc.StreamingCall call;
1122+
try {
1123+
call =
1124+
rpc.executeQuery(
1125+
request.build(),
1126+
stream.consumer(),
1127+
getTransactionChannelHint(),
1128+
requestId,
1129+
isRouteToLeader());
1130+
} catch (RuntimeException | Error t) {
1131+
onStartFailed(withBeginTransaction, t);
1132+
throw t;
1133+
}
11181134
session.markUsed(clock.instant());
1119-
stream.setCall(call, request.getTransaction().hasBegin());
1135+
stream.setCall(call, withBeginTransaction);
11201136
return stream;
11211137
}
11221138

@@ -1232,6 +1248,8 @@ public void onDone(boolean withBeginTransaction) {
12321248
this.session.onReadDone();
12331249
}
12341250

1251+
void onStartFailed(boolean withBeginTransaction, Throwable t) {}
1252+
12351253
/**
12361254
* For transactions other than read-write, the MultiplexedSessionPrecommitToken will not be
12371255
* present in the RPC response. In such cases, this method will be a no-op.
@@ -1331,15 +1349,22 @@ CloseableIterator<PartialResultSet> startStream(
13311349
builder.setTransaction(selector);
13321350
}
13331351
builder.setRequestOptions(buildRequestOptions(readOptions));
1334-
SpannerRpc.StreamingCall call =
1335-
rpc.read(
1336-
builder.build(),
1337-
stream.consumer(),
1338-
getTransactionChannelHint(),
1339-
requestId,
1340-
isRouteToLeader());
1352+
boolean withBeginTransaction = builder.getTransaction().hasBegin();
1353+
SpannerRpc.StreamingCall call;
1354+
try {
1355+
call =
1356+
rpc.read(
1357+
builder.build(),
1358+
stream.consumer(),
1359+
getTransactionChannelHint(),
1360+
requestId,
1361+
isRouteToLeader());
1362+
} catch (RuntimeException | Error t) {
1363+
onStartFailed(withBeginTransaction, t);
1364+
throw t;
1365+
}
13411366
session.markUsed(clock.instant());
1342-
stream.setCall(call, /* withBeginTransaction= */ builder.getTransaction().hasBegin());
1367+
stream.setCall(call, withBeginTransaction);
13431368
return stream;
13441369
}
13451370

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
import com.google.api.gax.rpc.ServerStream;
2020
import com.google.cloud.Timestamp;
21-
import com.google.cloud.spanner.Options.RpcPriority;
2221
import com.google.cloud.spanner.Options.ReadOnlyTransactionOption;
22+
import com.google.cloud.spanner.Options.RpcPriority;
2323
import com.google.cloud.spanner.Options.TransactionOption;
2424
import com.google.cloud.spanner.Options.UpdateOption;
2525
import com.google.cloud.spanner.Statement.StatementFactory;
@@ -353,9 +353,8 @@ ServerStream<BatchWriteResponse> batchWriteAtLeastOnce(
353353
ReadOnlyTransaction readOnlyTransaction();
354354

355355
/**
356-
* Returns a read-only transaction context in which multiple reads and/or queries can be
357-
* performed using {@link TimestampBound#strong()} concurrency and the given read-only
358-
* transaction options.
356+
* Returns a read-only transaction context in which multiple reads and/or queries can be performed
357+
* using {@link TimestampBound#strong()} concurrency and the given read-only transaction options.
359358
*
360359
* @param options options for starting the read-only transaction
361360
*/
@@ -397,8 +396,8 @@ default ReadOnlyTransaction readOnlyTransaction(ReadOnlyTransactionOption... opt
397396
ReadOnlyTransaction readOnlyTransaction(TimestampBound bound);
398397

399398
/**
400-
* Returns a read-only transaction context in which multiple reads and/or queries can be
401-
* performed at the given timestamp bound and with the given read-only transaction options.
399+
* Returns a read-only transaction context in which multiple reads and/or queries can be performed
400+
* at the given timestamp bound and with the given read-only transaction options.
402401
*
403402
* <p>Options can include:
404403
*
@@ -414,7 +413,8 @@ default ReadOnlyTransaction readOnlyTransaction(ReadOnlyTransactionOption... opt
414413
default ReadOnlyTransaction readOnlyTransaction(
415414
TimestampBound bound, ReadOnlyTransactionOption... options) {
416415
Options readOnlyTransactionOptions = Options.fromReadOnlyTransactionOptions(options);
417-
if (readOnlyTransactionOptions.beginTransactionOption() == Options.BeginTransactionOption.EXPLICIT) {
416+
if (readOnlyTransactionOptions.beginTransactionOption()
417+
== Options.BeginTransactionOption.EXPLICIT) {
418418
return readOnlyTransaction(bound);
419419
}
420420
throw new UnsupportedOperationException(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ public ReadOnlyTransaction readOnlyTransaction(ReadOnlyTransactionOption... opti
229229

230230
@Override
231231
public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) {
232-
return readOnlyTransaction(bound, Options.beginTransactionOption(Options.BeginTransactionOption.EXPLICIT));
232+
return readOnlyTransaction(
233+
bound, Options.beginTransactionOption(Options.BeginTransactionOption.EXPLICIT));
233234
}
234235

235236
@Override

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@
1919
import static com.google.cloud.spanner.SessionImpl.NO_CHANNEL_HINT;
2020

2121
import com.google.api.core.ApiFuture;
22-
import com.google.cloud.spanner.Options.ReadOnlyTransactionOption;
2322
import com.google.api.core.ApiFutures;
2423
import com.google.api.gax.rpc.ServerStream;
2524
import com.google.cloud.Timestamp;
2625
import com.google.cloud.spanner.DelayedReadContext.DelayedReadOnlyTransaction;
2726
import com.google.cloud.spanner.MultiplexedSessionDatabaseClient.MultiplexedSessionTransaction;
27+
import com.google.cloud.spanner.Options.ReadOnlyTransactionOption;
2828
import com.google.cloud.spanner.Options.TransactionOption;
2929
import com.google.cloud.spanner.Options.UpdateOption;
3030
import com.google.common.util.concurrent.MoreExecutors;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
import static com.google.cloud.spanner.SessionImpl.NO_CHANNEL_HINT;
2020

2121
import com.google.api.core.ApiFuture;
22-
import com.google.cloud.spanner.Options.ReadOnlyTransactionOption;
2322
import com.google.api.core.ApiFutures;
2423
import com.google.api.core.SettableApiFuture;
2524
import com.google.api.gax.rpc.ServerStream;
2625
import com.google.cloud.Timestamp;
26+
import com.google.cloud.spanner.Options.ReadOnlyTransactionOption;
2727
import com.google.cloud.spanner.Options.TransactionOption;
2828
import com.google.cloud.spanner.Options.UpdateOption;
2929
import com.google.cloud.spanner.SessionClient.SessionConsumer;

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,8 @@ RequestOptions toRequestOptionsProto(boolean isTransactionOption) {
316316
}
317317

318318
/**
319-
* Specifies how a multi-use read-only transaction should be started. The default is
320-
* {@link BeginTransactionOption#EXPLICIT}.
319+
* Specifies how a multi-use read-only transaction should be started. The default is {@link
320+
* BeginTransactionOption#EXPLICIT}.
321321
*/
322322
public static ReadOnlyTransactionOption beginTransactionOption(BeginTransactionOption option) {
323323
return new BeginTransactionOptionOption(Preconditions.checkNotNull(option));
@@ -833,7 +833,9 @@ ReadLockMode readLockMode() {
833833
}
834834

835835
BeginTransactionOption beginTransactionOption() {
836-
return beginTransactionOption == null ? BeginTransactionOption.EXPLICIT : beginTransactionOption;
836+
return beginTransactionOption == null
837+
? BeginTransactionOption.EXPLICIT
838+
: beginTransactionOption;
837839
}
838840

839841
@Override

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -635,8 +635,7 @@ public void multiUseReadOnlyTransactionCanUseInlineBegin() throws ParseException
635635
PartialResultSet resultSet =
636636
PartialResultSet.newBuilder()
637637
.setMetadata(
638-
newMetadata(Type.struct(Type.StructField.of("C", Type.string())))
639-
.toBuilder()
638+
newMetadata(Type.struct(Type.StructField.of("C", Type.string()))).toBuilder()
640639
.setTransaction(
641640
Transaction.newBuilder()
642641
.setId(ByteString.copyFromUtf8("inline-tx"))
@@ -665,7 +664,9 @@ public void multiUseReadOnlyTransactionCanUseInlineBegin() throws ParseException
665664
Mockito.verify(rpc, Mockito.never()).beginTransaction(Mockito.any(), anyMap(), eq(false));
666665
assertEquals(2, request.getAllValues().size());
667666
assertThat(request.getAllValues().get(0).getTransaction().hasBegin()).isTrue();
668-
assertEquals(ByteString.copyFromUtf8("inline-tx"), request.getAllValues().get(1).getTransaction().getId());
667+
assertEquals(
668+
ByteString.copyFromUtf8("inline-tx"),
669+
request.getAllValues().get(1).getTransaction().getId());
669670
}
670671

671672
@Test

0 commit comments

Comments
 (0)