Skip to content

Commit 71cda80

Browse files
odeke-emodeke-em
authored
chore(x-goog-spanner-request-id): setup expectations for end-to-end request-id checks (#3932)
* chore(x-goog-spanner-request-id): setup expectations for end-to-end request-id checks This change allows us to perform future end-to-end checks to assert on the behaviour of x-goog-spanner-request-id header values. Updates #3537 * Suffix checks * Fix up slicing in comparison checks
1 parent 675e90b commit 71cda80

File tree

6 files changed

+231
-28
lines changed

6 files changed

+231
-28
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
@VisibleForTesting final MultiplexedSessionDatabaseClient multiplexedSessionDatabaseClient;
4949
@VisibleForTesting final boolean useMultiplexedSessionPartitionedOps;
5050
@VisibleForTesting final boolean useMultiplexedSessionForRW;
51-
private final int dbId;
51+
@VisibleForTesting final int dbId;
5252
private final AtomicInteger nthRequest;
5353
private final Map<String, Integer> clientIdToOrdinalMap;
5454

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,9 +1098,6 @@ public boolean equals(Object o) {
10981098
return false;
10991099
}
11001100
RequestIdOption other = (RequestIdOption) o;
1101-
if (this.reqId == null || other.reqId == null) {
1102-
return this.reqId == null && other.reqId == null;
1103-
}
11041101
return Objects.equals(this.reqId, other.reqId);
11051102
}
11061103
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ DatabaseId getDatabaseId() {
223223
@Override
224224
public XGoogSpannerRequestId nextRequestId(long channelId, int attempt) {
225225
return XGoogSpannerRequestId.of(
226-
this.nthId, this.nthRequest.incrementAndGet(), channelId, attempt);
226+
this.nthId, channelId, this.nthRequest.incrementAndGet(), attempt);
227227
}
228228

229229
/** Create a single session. */
@@ -423,7 +423,7 @@ private List<SessionImpl> internalBatchCreateSessions(
423423
span.addAnnotation(String.format("Requesting %d sessions", sessionCount));
424424
try (IScope s = spanner.getTracer().withSpan(span)) {
425425
XGoogSpannerRequestId reqId =
426-
XGoogSpannerRequestId.of(this.nthId, this.nthRequest.incrementAndGet(), channelHint, 1);
426+
XGoogSpannerRequestId.of(this.nthId, channelHint, this.nthRequest.incrementAndGet(), 1);
427427
List<com.google.spanner.v1.Session> sessions =
428428
spanner
429429
.getRpc()

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,6 +2042,7 @@ <ReqT, RespT> GrpcCallContext newCallContext(
20422042
}
20432043
}
20442044
if (options != null) {
2045+
// TODO(@odeke-em): Infer the affinity if it doesn't match up with in the request-id.
20452046
context = withRequestId(context, options);
20462047
}
20472048
context = context.withExtraHeaders(metadataProvider.newExtraHeaders(resource, projectName));

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2907,6 +2907,37 @@ public void testPartitionedDmlDoesNotTimeout() {
29072907
return null;
29082908
}));
29092909
assertEquals(ErrorCode.DEADLINE_EXCEEDED, e.getErrorCode());
2910+
2911+
DatabaseClientImpl dbImpl = ((DatabaseClientImpl) client);
2912+
int channelId = 0;
2913+
try (Session session = dbImpl.getSession()) {
2914+
channelId = ((PooledSessionFuture) session).getChannel();
2915+
}
2916+
int dbId = dbImpl.dbId;
2917+
long NON_DETERMINISTIC = XGoogSpannerRequestIdTest.NON_DETERMINISTIC;
2918+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantStreamingValues = {
2919+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
2920+
"google.spanner.v1.Spanner/ExecuteStreamingSql",
2921+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 6, 1)),
2922+
};
2923+
if (false) { // TODO(@odeke-em): enable in next PRs.
2924+
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
2925+
}
2926+
2927+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantUnaryValues = {
2928+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
2929+
"google.spanner.v1.Spanner/BeginTransaction",
2930+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 7, 1)),
2931+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
2932+
"google.spanner.v1.Spanner/CreateSession",
2933+
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
2934+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
2935+
"google.spanner.v1.Spanner/ExecuteSql",
2936+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 8, 1)),
2937+
};
2938+
if (false) { // TODO(@odeke-em): enable in next PRs.
2939+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
2940+
}
29102941
}
29112942
}
29122943

@@ -2989,6 +3020,38 @@ public void testPartitionedDmlWithHigherTimeout() {
29893020
.run(transaction -> transaction.executeUpdate(UPDATE_STATEMENT)));
29903021
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.DEADLINE_EXCEEDED);
29913022
assertThat(updateCount).isEqualTo(UPDATE_COUNT);
3023+
3024+
DatabaseClientImpl dbImpl = ((DatabaseClientImpl) client);
3025+
int channelId = 0;
3026+
try (Session session = dbImpl.getSession()) {
3027+
channelId = ((PooledSessionFuture) session).getChannel();
3028+
}
3029+
int dbId = dbImpl.dbId;
3030+
long NON_DETERMINISTIC = XGoogSpannerRequestIdTest.NON_DETERMINISTIC;
3031+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantStreamingValues = {
3032+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
3033+
"google.spanner.v1.Spanner/ExecuteStreamingSql",
3034+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 6, 1)),
3035+
};
3036+
3037+
if (false) { // TODO(@odeke-em): enable in next PRs.
3038+
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
3039+
}
3040+
3041+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantUnaryValues = {
3042+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
3043+
"google.spanner.v1.Spanner/BeginTransaction",
3044+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 7, 1)),
3045+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
3046+
"google.spanner.v1.Spanner/CreateSession",
3047+
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
3048+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
3049+
"google.spanner.v1.Spanner/ExecuteSql",
3050+
new XGoogSpannerRequestId(NON_DETERMINISTIC, channelId, 8, 1)),
3051+
};
3052+
if (false) { // TODO(@odeke-em): enable in next PRs.
3053+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
3054+
}
29923055
}
29933056
}
29943057

@@ -5314,6 +5377,26 @@ public void testSessionPoolExhaustedError_containsStackTraces() {
53145377
}
53155378
// Closing the transactions should return the sessions to the pool.
53165379
assertEquals(4, pool.getNumberOfSessionsInPool());
5380+
5381+
DatabaseClientImpl dbClient = (DatabaseClientImpl) client;
5382+
int channelId = 0;
5383+
try (Session session = dbClient.getSession()) {
5384+
channelId = ((PooledSessionFuture) session).getChannel();
5385+
}
5386+
int dbId = dbClient.dbId;
5387+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantStreamingValues = {};
5388+
5389+
xGoogReqIdInterceptor.checkExpectedStreamingXGoogRequestIds(wantStreamingValues);
5390+
long NON_DETERMINISTIC = XGoogSpannerRequestIdTest.NON_DETERMINISTIC;
5391+
5392+
XGoogSpannerRequestIdTest.MethodAndRequestId[] wantUnaryValues = {
5393+
XGoogSpannerRequestIdTest.ofMethodAndRequestId(
5394+
"google.spanner.v1.Spanner/CreateSession",
5395+
new XGoogSpannerRequestId(NON_DETERMINISTIC, 0, 1, 1)),
5396+
};
5397+
if (false) { // TODO(@odeke-em): enable in next PRs.
5398+
xGoogReqIdInterceptor.checkExpectedUnaryXGoogRequestIdsAsSuffixes(wantUnaryValues);
5399+
}
53175400
}
53185401
}
53195402

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

Lines changed: 144 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import io.grpc.ServerInterceptor;
2929
import io.grpc.Status;
3030
import java.util.ArrayList;
31+
import java.util.Arrays;
32+
import java.util.Comparator;
3133
import java.util.List;
3234
import java.util.Map;
3335
import java.util.Objects;
@@ -41,6 +43,7 @@
4143

4244
@RunWith(JUnit4.class)
4345
public class XGoogSpannerRequestIdTest {
46+
public static long NON_DETERMINISTIC = -1;
4447

4548
@Test
4649
public void testEquals() {
@@ -157,40 +160,83 @@ private void assertMonotonicityOfIds(String prefix, List<XGoogSpannerRequestId>
157160
+ String.join("\n\t", violations.toArray(new String[0])));
158161
}
159162

160-
public static class methodAndRequestId {
161-
String method;
162-
String requestId;
163-
164-
public methodAndRequestId(String method, String requestId) {
165-
this.method = method;
166-
this.requestId = requestId;
167-
}
168-
169-
public String toString() {
170-
return "{" + this.method + ":" + this.requestId + "}";
171-
}
172-
}
173-
174-
public methodAndRequestId[] accumulatedUnaryValues() {
175-
List<methodAndRequestId> accumulated = new ArrayList();
163+
public MethodAndRequestId[] accumulatedUnaryValues() {
164+
List<MethodAndRequestId> accumulated = new ArrayList();
176165
this.unaryResults.forEach(
177166
(String method, CopyOnWriteArrayList<XGoogSpannerRequestId> values) -> {
178167
for (int i = 0; i < values.size(); i++) {
179-
accumulated.add(new methodAndRequestId(method, values.get(i).toString()));
168+
accumulated.add(new MethodAndRequestId(method, values.get(i)));
180169
}
181170
});
182-
return accumulated.toArray(new methodAndRequestId[0]);
171+
return accumulated.toArray(new MethodAndRequestId[0]);
183172
}
184173

185-
public methodAndRequestId[] accumulatedStreamingValues() {
186-
List<methodAndRequestId> accumulated = new ArrayList();
174+
public MethodAndRequestId[] accumulatedStreamingValues() {
175+
List<MethodAndRequestId> accumulated = new ArrayList();
187176
this.streamingResults.forEach(
188177
(String method, CopyOnWriteArrayList<XGoogSpannerRequestId> values) -> {
189178
for (int i = 0; i < values.size(); i++) {
190-
accumulated.add(new methodAndRequestId(method, values.get(i).toString()));
179+
accumulated.add(new MethodAndRequestId(method, values.get(i)));
191180
}
192181
});
193-
return accumulated.toArray(new methodAndRequestId[0]);
182+
return accumulated.toArray(new MethodAndRequestId[0]);
183+
}
184+
185+
public void checkExpectedUnaryXGoogRequestIds(MethodAndRequestId... wantUnaryValues) {
186+
MethodAndRequestId[] gotUnaryValues = this.accumulatedUnaryValues();
187+
sortValues(gotUnaryValues);
188+
for (int i = 0; i < gotUnaryValues.length && false; i++) {
189+
System.out.println("\033[33misUnary: #" + i + ":: " + gotUnaryValues[i] + "\033[00m");
190+
}
191+
assertEquals(wantUnaryValues, gotUnaryValues);
192+
}
193+
194+
public void checkAtLeastHasExpectedUnaryXGoogRequestIds(MethodAndRequestId... wantUnaryValues) {
195+
MethodAndRequestId[] gotUnaryValues = this.accumulatedUnaryValues();
196+
sortValues(gotUnaryValues);
197+
for (int i = 0; i < gotUnaryValues.length && false; i++) {
198+
System.out.println("\033[33misUnary: #" + i + ":: " + gotUnaryValues[i] + "\033[00m");
199+
}
200+
if (wantUnaryValues.length < gotUnaryValues.length) {
201+
MethodAndRequestId[] gotSliced =
202+
Arrays.copyOfRange(gotUnaryValues, 0, wantUnaryValues.length);
203+
assertEquals(wantUnaryValues, gotSliced);
204+
} else {
205+
assertEquals(wantUnaryValues, gotUnaryValues);
206+
}
207+
}
208+
209+
public void checkExpectedUnaryXGoogRequestIdsAsSuffixes(MethodAndRequestId... wantUnaryValues) {
210+
MethodAndRequestId[] gotUnaryValues = this.accumulatedUnaryValues();
211+
sortValues(gotUnaryValues);
212+
for (int i = 0; i < gotUnaryValues.length && false; i++) {
213+
System.out.println("\033[33misUnary: #" + i + ":: " + gotUnaryValues[i] + "\033[00m");
214+
}
215+
if (wantUnaryValues.length < gotUnaryValues.length) {
216+
MethodAndRequestId[] gotSliced =
217+
Arrays.copyOfRange(
218+
gotUnaryValues,
219+
gotUnaryValues.length - wantUnaryValues.length,
220+
gotUnaryValues.length);
221+
assertEquals(wantUnaryValues, gotSliced);
222+
} else {
223+
assertEquals(wantUnaryValues, gotUnaryValues);
224+
}
225+
}
226+
227+
private void sortValues(MethodAndRequestId[] values) {
228+
massageValues(values);
229+
Arrays.sort(values, new MethodAndRequestIdComparator());
230+
}
231+
232+
public void checkExpectedStreamingXGoogRequestIds(MethodAndRequestId... wantStreamingValues) {
233+
MethodAndRequestId[] gotStreamingValues = this.accumulatedStreamingValues();
234+
for (int i = 0; i < gotStreamingValues.length && false; i++) {
235+
System.out.println(
236+
"\033[32misStreaming: #" + i + ":: " + gotStreamingValues[i] + "\033[00m");
237+
}
238+
sortValues(gotStreamingValues);
239+
assertEquals(wantStreamingValues, gotStreamingValues);
194240
}
195241

196242
public void reset() {
@@ -199,4 +245,80 @@ public void reset() {
199245
this.streamingResults.clear();
200246
}
201247
}
248+
249+
public static class MethodAndRequestId {
250+
String method;
251+
XGoogSpannerRequestId requestId;
252+
253+
public MethodAndRequestId(String method, XGoogSpannerRequestId requestId) {
254+
this.method = method;
255+
this.requestId = requestId;
256+
}
257+
258+
public String toString() {
259+
return "{" + this.method + ":" + this.requestId.debugToString() + "}";
260+
}
261+
262+
@Override
263+
public boolean equals(Object o) {
264+
if (!(o instanceof MethodAndRequestId)) {
265+
return false;
266+
}
267+
MethodAndRequestId other = (MethodAndRequestId) o;
268+
return Objects.equals(this.method, other.method)
269+
&& Objects.equals(this.requestId, other.requestId);
270+
}
271+
}
272+
273+
static class MethodAndRequestIdComparator implements Comparator<MethodAndRequestId> {
274+
@Override
275+
public int compare(MethodAndRequestId mr1, MethodAndRequestId mr2) {
276+
int cmpMethod = mr1.method.compareTo(mr2.method);
277+
if (cmpMethod != 0) {
278+
return cmpMethod;
279+
}
280+
281+
if (Objects.equals(mr1.requestId, mr2.requestId)) {
282+
return 0;
283+
}
284+
if (mr1.requestId.isGreaterThan(mr2.requestId)) {
285+
return +1;
286+
}
287+
return -1;
288+
}
289+
}
290+
291+
static void massageValues(MethodAndRequestId[] mreqs) {
292+
for (int i = 0; i < mreqs.length; i++) {
293+
MethodAndRequestId mreq = mreqs[i];
294+
// BatchCreateSessions is so hard to control as the round-robin doling out
295+
// hence we might need to be able to scrub the nth_request that won't match
296+
// nth_req in consecutive order of nth_client.
297+
if (mreq.method.compareTo("google.spanner.v1.Spanner/BatchCreateSessions") == 0) {
298+
mreqs[i] =
299+
new MethodAndRequestId(
300+
mreq.method,
301+
mreq.requestId
302+
.withNthRequest(NON_DETERMINISTIC)
303+
.withChannelId(NON_DETERMINISTIC)
304+
.withNthClientId(NON_DETERMINISTIC));
305+
} else if (mreq.method.compareTo("google.spanner.v1.Spanner/BeginTransaction") == 0
306+
|| mreq.method.compareTo("google.spanner.v1.Spanner/ExecuteStreamingSql") == 0
307+
|| mreq.method.compareTo("google.spanner.v1.Spanner/ExecuteSql") == 0
308+
|| mreq.method.compareTo("google.spanner.v1.Spanner/CreateSession") == 0
309+
|| mreq.method.compareTo("google.spanner.v1.Spanner/Commit") == 0) {
310+
mreqs[i] =
311+
new MethodAndRequestId(mreq.method, mreq.requestId.withNthClientId(NON_DETERMINISTIC));
312+
}
313+
}
314+
}
315+
316+
public static MethodAndRequestId ofMethodAndRequestId(String method, String reqId) {
317+
return new MethodAndRequestId(method, XGoogSpannerRequestId.of(reqId));
318+
}
319+
320+
public static MethodAndRequestId ofMethodAndRequestId(
321+
String method, XGoogSpannerRequestId reqId) {
322+
return new MethodAndRequestId(method, reqId);
323+
}
202324
}

0 commit comments

Comments
 (0)