Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

Commit 7c421f2

Browse files
tharoldDtharoldD
committed
sort requestids before checking monotonicity
1 parent f00a453 commit 7c421f2

3 files changed

Lines changed: 50 additions & 39 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,4 +199,10 @@ XGoogSpannerRequestId withNthClientId(long replacementClientId) {
199199
return XGoogSpannerRequestId.of(
200200
replacementClientId, this.nthChannelId, this.nthRequest, this.attempt);
201201
}
202+
203+
@VisibleForTesting
204+
XGoogSpannerRequestId withAttempt(long replacementAttempt) {
205+
return XGoogSpannerRequestId.of(
206+
this.nthClientId, this.nthChannelId, this.nthRequest, replacementAttempt);
207+
}
202208
}

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

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,8 +1417,7 @@ public void testWriteAtLeastOnceAborted() {
14171417

14181418
List<CommitRequest> commitRequests = mockSpanner.getRequestsOfType(CommitRequest.class);
14191419
assertEquals(2, commitRequests.size());
1420-
// TODO(@odeke-em): Enable in later PR.
1421-
// xGoogReqIdInterceptor.assertIntegrity();
1420+
xGoogReqIdInterceptor.assertIntegrity();
14221421
}
14231422

14241423
@Test
@@ -2910,34 +2909,34 @@ public void testPartitionedDmlDoesNotTimeout() {
29102909

29112910
DatabaseClientImpl dbImpl = ((DatabaseClientImpl) client);
29122911
int channelId = 0;
2913-
try (Session session = dbImpl.getSession()) {
2914-
channelId = ((PooledSessionFuture) session).getChannel();
2912+
try (PooledSessionFuture session = dbImpl.getSession()) {
2913+
channelId = session.getChannel();
29152914
}
29162915
int dbId = dbImpl.dbId;
29172916
long NON_DETERMINISTIC = XGoogSpannerRequestIdTest.NON_DETERMINISTIC;
29182917
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantStreamingValues = {
29192918
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
29202919
"google.spanner.v1.Spanner/ExecuteStreamingSql",
2921-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 6, 1)),
2920+
new XGoogSpannerRequestId(
2921+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
29222922
};
2923-
if (false) { // TODO(@odeke-em): enable in next PRs.
2924-
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
2925-
}
2923+
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
29262924

29272925
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantUnaryValues = {
29282926
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
29292927
"google.spanner.v1.Spanner/BeginTransaction",
2930-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 7, 1)),
2928+
new XGoogSpannerRequestId(
2929+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
29312930
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
29322931
"google.spanner.v1.Spanner/CreateSession",
2933-
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
2932+
new XGoogSpannerRequestId(
2933+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
29342934
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
29352935
"google.spanner.v1.Spanner/ExecuteSql",
2936-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 8, 1)),
2936+
new XGoogSpannerRequestId(
2937+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
29372938
};
2938-
if (false) { // TODO(@odeke-em): enable in next PRs.
2939-
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
2940-
}
2939+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
29412940
}
29422941
}
29432942

@@ -3023,35 +3022,35 @@ public void testPartitionedDmlWithHigherTimeout() {
30233022

30243023
DatabaseClientImpl dbImpl = ((DatabaseClientImpl) client);
30253024
int channelId = 0;
3026-
try (Session session = dbImpl.getSession()) {
3027-
channelId = ((PooledSessionFuture) session).getChannel();
3025+
try (PooledSessionFuture session = dbImpl.getSession()) {
3026+
channelId = session.getChannel();
30283027
}
30293028
int dbId = dbImpl.dbId;
30303029
long NON_DETERMINISTIC = XGoogSpannerRequestIdTest.NON_DETERMINISTIC;
30313030
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantStreamingValues = {
30323031
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
30333032
"google.spanner.v1.Spanner/ExecuteStreamingSql",
3034-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 6, 1)),
3033+
new XGoogSpannerRequestId(
3034+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
30353035
};
30363036

3037-
if (false) { // TODO(@odeke-em): enable in next PRs.
3038-
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
3039-
}
3037+
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
30403038

30413039
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantUnaryValues = {
30423040
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
30433041
"google.spanner.v1.Spanner/BeginTransaction",
3044-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 7, 1)),
3042+
new XGoogSpannerRequestId(
3043+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
30453044
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
30463045
"google.spanner.v1.Spanner/CreateSession",
3047-
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
3046+
new XGoogSpannerRequestId(
3047+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
30483048
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
30493049
"google.spanner.v1.Spanner/ExecuteSql",
3050-
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 8, 1)),
3050+
new XGoogSpannerRequestId(
3051+
NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC, NON_DETERMINISTIC)),
30513052
};
3052-
if (false) { // TODO(@odeke-em): enable in next PRs.
3053-
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
3054-
}
3053+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
30553054
}
30563055
}
30573056

@@ -5304,8 +5303,7 @@ public void testSelectHasXGoogRequestIdHeader() {
53045303
assertEquals(1L, resultSet.getLong(0));
53055304
assertFalse(resultSet.next());
53065305
} finally {
5307-
// TODO(@odeke-em): Enable in later PR.
5308-
// xGoogReqIdInterceptor.assertIntegrity();
5306+
xGoogReqIdInterceptor.assertIntegrity();
53095307
}
53105308
}
53115309

@@ -5394,9 +5392,7 @@ public void testSessionPoolExhaustedError_containsStackTraces() {
53945392
"google.spanner.v1.Spanner/CreateSession",
53955393
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
53965394
};
5397-
if (false) { // TODO(@odeke-em): enable in next PRs.
5398-
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
5399-
}
5395+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
54005396
}
54015397
}
54025398

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,9 @@ public static class ServerHeaderEnforcer implements ServerInterceptor {
7373
private Set<String> checkMethods;
7474

7575
ServerHeaderEnforcer(Set<String> checkMethods) {
76-
this.gotValues = new CopyOnWriteArrayList<String>();
77-
this.unaryResults =
78-
new ConcurrentHashMap<String, CopyOnWriteArrayList<XGoogSpannerRequestId>>();
79-
this.streamingResults =
80-
new ConcurrentHashMap<String, CopyOnWriteArrayList<XGoogSpannerRequestId>>();
76+
this.gotValues = new CopyOnWriteArrayList<>();
77+
this.unaryResults = new ConcurrentHashMap<>();
78+
this.streamingResults = new ConcurrentHashMap<>();
8179
this.checkMethods = checkMethods;
8280
}
8381

@@ -139,6 +137,11 @@ public void assertIntegrity() {
139137
}
140138

141139
private void assertMonotonicityOfIds(String prefix, List<XGoogSpannerRequestId> reqIds) {
140+
reqIds.sort(
141+
(id1, id2) -> {
142+
if (id1.equals(id2)) return 0;
143+
return id1.isGreaterThan(id2) ? 1 : -1;
144+
});
142145
int size = reqIds.size();
143146

144147
List<String> violations = new ArrayList<>();
@@ -161,7 +164,7 @@ private void assertMonotonicityOfIds(String prefix, List<XGoogSpannerRequestId>
161164
}
162165

163166
public MethodAndRequestId[] accumulatedUnaryValues() {
164-
List<MethodAndRequestId> accumulated = new ArrayList();
167+
List<MethodAndRequestId> accumulated = new ArrayList<>();
165168
this.unaryResults.forEach(
166169
(String method, CopyOnWriteArrayList<XGoogSpannerRequestId> values) -> {
167170
for (int i = 0; i < values.size(); i++) {
@@ -172,7 +175,7 @@ public MethodAndRequestId[] accumulatedUnaryValues() {
172175
}
173176

174177
public MethodAndRequestId[] accumulatedStreamingValues() {
175-
List<MethodAndRequestId> accumulated = new ArrayList();
178+
List<MethodAndRequestId> accumulated = new ArrayList<>();
176179
this.streamingResults.forEach(
177180
(String method, CopyOnWriteArrayList<XGoogSpannerRequestId> values) -> {
178181
for (int i = 0; i < values.size(); i++) {
@@ -308,7 +311,13 @@ static void massageValues(MethodAndRequestId[] mreqs) {
308311
|| mreq.method.compareTo("google.spanner.v1.Spanner/CreateSession") == 0
309312
|| mreq.method.compareTo("google.spanner.v1.Spanner/Commit") == 0) {
310313
mreqs[i] =
311-
new MethodAndRequestId(mreq.method, mreq.requestId.withNthClientId(NON_DETERMINISTIC));
314+
new MethodAndRequestId(
315+
mreq.method,
316+
mreq.requestId
317+
.withNthClientId(NON_DETERMINISTIC)
318+
.withChannelId(NON_DETERMINISTIC)
319+
.withNthRequest(NON_DETERMINISTIC)
320+
.withAttempt(NON_DETERMINISTIC));
312321
}
313322
}
314323
}

0 commit comments

Comments
 (0)