Skip to content

Commit 46108f4

Browse files
authored
perf(spanner): stop re-parsing the request id on every RPC (#14353)
RequestIdTargetTracker keys its cache on the logical request key, but it accepted the x-goog-spanner-request-id header value and derived that key by calling XGoogSpannerRequestId.of(String), which runs a six-group regex plus four Long.parseLong calls. Every caller already holds the parsed XGoogSpannerRequestId in the gRPC CallOptions. HeaderInterceptor paid this twice per RPC (a get on the first response and a remove on close). KeyAwareChannel added a third by rendering the object it already held back into a header string with getHeaderValue(), only for the tracker to parse it again. Location-aware routing is the only producer of tracked targets and is disabled unless the instance type is OMNI, so for most clients the cache stays empty for the lifetime of the process and all of that work is wasted. The tracker now accepts the parsed request id (or the logical key, where the caller already has one) and short circuits on a volatile flag that is set the first time a target is recorded. This also fixes a latent bug in KeyAwareChannel.onClose, which passed an already-normalized key into remove(String). The regex never matched it, so every RPC on that path constructed and threw an IllegalStateException, including its concatenated message and stack trace fill-in, before the catch block fell back to returning the raw string. Measured per RPC (JMH, 2 forks x 8 x 1s, -prof gc): ``` before 581.2 ns 1376 B after, default config 2.0 ns 0 B after, location-aware on 42.5 ns 56 B ```
1 parent 7de24de commit 46108f4

5 files changed

Lines changed: 243 additions & 49 deletions

File tree

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static com.google.api.gax.grpc.GrpcCallContext.TRACER_KEY;
1919
import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID;
20+
import static com.google.cloud.spanner.XGoogSpannerRequestId.REQUEST_ID_CALL_OPTIONS_KEY;
2021
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.DATABASE_ID;
2122
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.INSTANCE_ID;
2223
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.METHOD;
@@ -111,6 +112,9 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
111112
ApiTracer tracer = callOptions.getOption(TRACER_KEY);
112113
CompositeTracer compositeTracer =
113114
tracer instanceof CompositeTracer ? (CompositeTracer) tracer : null;
115+
// The same request id that RequestIdInterceptor writes to the request header. Reading it from
116+
// the call options avoids having to parse the header value back into an object on every RPC.
117+
XGoogSpannerRequestId parsedRequestId = callOptions.getOption(REQUEST_ID_CALL_OPTIONS_KEY);
114118
return new SimpleForwardingClientCall<ReqT, RespT>(next.newCall(method, callOptions)) {
115119
@Override
116120
public void start(Listener<RespT> responseListener, Metadata headers) {
@@ -133,7 +137,8 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
133137
@Override
134138
public void onHeaders(Metadata metadata) {
135139
try {
136-
recordFirstResponseLatency(requestId, startedAtNanos, firstResponseRecorded);
140+
recordFirstResponseLatency(
141+
parsedRequestId, startedAtNanos, firstResponseRecorded);
137142
String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY);
138143
try {
139144
// Get gfe and afe Latency value
@@ -170,7 +175,8 @@ public void onClose(Status status, Metadata trailers) {
170175
LEVEL, "Unable to get built-in metric attributes {0}", e.getMessage());
171176
}
172177
if (status.isOk()) {
173-
recordFirstResponseLatency(requestId, startedAtNanos, firstResponseRecorded);
178+
recordFirstResponseLatency(
179+
parsedRequestId, startedAtNanos, firstResponseRecorded);
174180
}
175181
recordBuiltInMetrics(
176182
compositeTracer,
@@ -183,7 +189,7 @@ public void onClose(Status status, Metadata trailers) {
183189
} catch (Throwable throwable) {
184190
LOGGER.log(Level.WARNING, "Error recording metrics in onClose", throwable);
185191
} finally {
186-
RequestIdTargetTracker.remove(requestId);
192+
RequestIdTargetTracker.remove(parsedRequestId);
187193
super.onClose(status, trailers);
188194
}
189195
}
@@ -263,7 +269,9 @@ private void recordBuiltInMetrics(
263269
}
264270

265271
private void recordFirstResponseLatency(
266-
String requestId, long startedAtNanos, AtomicBoolean firstResponseRecorded) {
272+
@Nullable XGoogSpannerRequestId requestId,
273+
long startedAtNanos,
274+
AtomicBoolean firstResponseRecorded) {
267275
if (!firstResponseRecorded.compareAndSet(false, true)) {
268276
return;
269277
}

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -913,15 +913,12 @@ public void sendMessage(RequestT message) {
913913
selectedPreferLeader = preferLeader;
914914
this.channelFinder = finder;
915915
selectedEndpoint.incrementActiveRequests();
916-
XGoogSpannerRequestId requestId = callOptions.getOption(REQUEST_ID_CALL_OPTIONS_KEY);
917-
if (requestId != null) {
918-
RequestIdTargetTracker.record(
919-
requestId.getHeaderValue(),
920-
selectedDatabaseScope,
921-
selectedTargetEndpoint,
922-
operationUid,
923-
selectedPreferLeader);
924-
}
916+
RequestIdTargetTracker.record(
917+
logicalRequestKey,
918+
selectedDatabaseScope,
919+
selectedTargetEndpoint,
920+
operationUid,
921+
selectedPreferLeader);
925922

926923
// Record real traffic for idle eviction tracking.
927924
parentChannel.onRequestRouted(endpoint);
@@ -1105,11 +1102,7 @@ boolean maybeRerouteUnimplementedGenericCall(io.grpc.Status status) {
11051102
selectedPreferLeader = false;
11061103
channelFinder = null;
11071104
selectedEndpoint.incrementActiveRequests();
1108-
XGoogSpannerRequestId requestId = callOptions.getOption(REQUEST_ID_CALL_OPTIONS_KEY);
1109-
if (requestId != null) {
1110-
RequestIdTargetTracker.record(
1111-
requestId.getHeaderValue(), null, selectedTargetEndpoint, 0L, false);
1112-
}
1105+
RequestIdTargetTracker.record(logicalRequestKey, null, selectedTargetEndpoint, 0L, false);
11131106
parentChannel.onRequestRouted(defaultEndpoint);
11141107
recordRouteSelectionTrace(methodDescriptor, defaultEndpoint.getAddress(), true, false);
11151108

@@ -1326,7 +1319,7 @@ public void onClose(io.grpc.Status status, Metadata trailers) {
13261319
if (call.selectedEndpoint != null) {
13271320
call.selectedEndpoint.decrementActiveRequests();
13281321
}
1329-
RequestIdTargetTracker.remove(call.logicalRequestKey);
1322+
RequestIdTargetTracker.removeLogicalKey(call.logicalRequestKey);
13301323
call.maybeClearAffinity();
13311324
super.onClose(status, trailers);
13321325
}

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

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@
2323
import java.util.concurrent.TimeUnit;
2424
import javax.annotation.Nullable;
2525

26+
/**
27+
* Associates an in-flight request with the endpoint that location-aware routing selected for it, so
28+
* that {@link HeaderInterceptor} can attribute the observed latency back to that endpoint.
29+
*
30+
* <p>Entries are keyed by {@link XGoogSpannerRequestId#getLogicalRequestKey()}, which is stable
31+
* across all attempts of the same logical RPC. Callers pass that key directly rather than the
32+
* {@code x-goog-spanner-request-id} header value: every call site either holds the parsed {@link
33+
* XGoogSpannerRequestId} (it is carried in the gRPC {@code CallOptions}) or has already derived the
34+
* key from it, so re-parsing the header string here would be pure overhead on the per-RPC path.
35+
*/
2636
final class RequestIdTargetTracker {
2737
@VisibleForTesting static final long MAX_TRACKED_TARGETS = 1_000_000L;
2838

@@ -37,54 +47,78 @@ final class RequestIdTargetTracker {
3747
.expireAfterWrite(10, TimeUnit.MINUTES)
3848
.build();
3949

50+
/**
51+
* Whether a routing target has ever been recorded. Location-aware routing is the only producer of
52+
* entries and is disabled unless the instance type is {@code OMNI}, so for most clients this
53+
* cache stays empty for the lifetime of the process. Checking this flag first lets the per-RPC
54+
* lookup and removal return immediately instead of probing the cache.
55+
*
56+
* <p>The flag only ever transitions from {@code false} to {@code true}, and every RPC reads it
57+
* from whichever thread it happens to run on. It is therefore written only when it is not already
58+
* set, so that recording a target does not repeatedly invalidate the cache line that all those
59+
* readers share. Visibility of the entries themselves does not depend on this flag; {@link Cache}
60+
* provides its own guarantees.
61+
*/
62+
private static volatile boolean tracking;
63+
4064
private RequestIdTargetTracker() {}
4165

4266
static void record(
43-
String requestId,
67+
@Nullable String logicalRequestKey,
4468
@Nullable String databaseScope,
45-
String targetEndpoint,
69+
@Nullable String targetEndpoint,
4670
long operationUid,
4771
boolean preferLeader) {
48-
String trackingKey = normalizeRequestKey(requestId);
49-
if (trackingKey == null || targetEndpoint == null || targetEndpoint.isEmpty()) {
72+
if (logicalRequestKey == null
73+
|| logicalRequestKey.isEmpty()
74+
|| targetEndpoint == null
75+
|| targetEndpoint.isEmpty()) {
5076
return;
5177
}
5278
TARGETS.put(
53-
trackingKey, new RoutingTarget(databaseScope, targetEndpoint, operationUid, preferLeader));
79+
logicalRequestKey,
80+
new RoutingTarget(databaseScope, targetEndpoint, operationUid, preferLeader));
81+
if (!tracking) {
82+
tracking = true;
83+
}
5484
}
5585

5686
@Nullable
57-
static RoutingTarget get(String requestId) {
58-
String trackingKey = normalizeRequestKey(requestId);
59-
if (trackingKey == null) {
60-
return null;
61-
}
62-
return TARGETS.getIfPresent(trackingKey);
87+
static RoutingTarget get(@Nullable XGoogSpannerRequestId requestId) {
88+
String logicalRequestKey = trackingKey(requestId);
89+
return logicalRequestKey == null ? null : TARGETS.getIfPresent(logicalRequestKey);
90+
}
91+
92+
static void remove(@Nullable XGoogSpannerRequestId requestId) {
93+
removeLogicalKey(trackingKey(requestId));
6394
}
6495

65-
static void remove(String requestId) {
66-
String trackingKey = normalizeRequestKey(requestId);
67-
if (trackingKey == null) {
96+
static void removeLogicalKey(@Nullable String logicalRequestKey) {
97+
if (!tracking || logicalRequestKey == null || logicalRequestKey.isEmpty()) {
6898
return;
6999
}
70-
TARGETS.invalidate(trackingKey);
100+
TARGETS.invalidate(logicalRequestKey);
101+
}
102+
103+
/**
104+
* Returns the cache key for {@code requestId}, or {@code null} if the key cannot be derived or
105+
* nothing is being tracked. Deriving the key allocates a string, so it is skipped entirely while
106+
* the cache is known to be empty.
107+
*/
108+
@Nullable
109+
private static String trackingKey(@Nullable XGoogSpannerRequestId requestId) {
110+
return tracking && requestId != null ? requestId.getLogicalRequestKey() : null;
71111
}
72112

73113
@VisibleForTesting
74-
static void clear() {
75-
TARGETS.invalidateAll();
114+
static boolean isTracking() {
115+
return tracking;
76116
}
77117

78118
@VisibleForTesting
79-
static String normalizeRequestKey(String requestId) {
80-
if (requestId == null || requestId.isEmpty()) {
81-
return null;
82-
}
83-
try {
84-
return XGoogSpannerRequestId.of(requestId).getLogicalRequestKey();
85-
} catch (IllegalStateException e) {
86-
return requestId;
87-
}
119+
static void clear() {
120+
TARGETS.invalidateAll();
121+
tracking = false;
88122
}
89123

90124
static final class RoutingTarget {

‎java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/HeaderInterceptorTest.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.cloud.spanner.spi.v1;
1818

1919
import static com.google.api.gax.grpc.GrpcCallContext.TRACER_KEY;
20+
import static com.google.cloud.spanner.XGoogSpannerRequestId.REQUEST_ID_CALL_OPTIONS_KEY;
2021
import static com.google.cloud.spanner.XGoogSpannerRequestId.REQUEST_ID_HEADER_KEY;
2122
import static org.junit.Assert.assertEquals;
2223
import static org.junit.Assert.assertNotNull;
@@ -25,6 +26,7 @@
2526

2627
import com.google.cloud.spanner.CompositeTracer;
2728
import com.google.cloud.spanner.SpannerRpcMetrics;
29+
import com.google.cloud.spanner.XGoogSpannerRequestId;
2830
import com.google.common.collect.ImmutableList;
2931
import io.grpc.CallOptions;
3032
import io.grpc.Channel;
@@ -170,18 +172,22 @@ public void recordServerTimingHeaderMetrics(
170172
}
171173
};
172174

173-
CallOptions callOptions = CallOptions.DEFAULT.withOption(TRACER_KEY, throwingTracer);
175+
XGoogSpannerRequestId requestId = XGoogSpannerRequestId.of(1, 1, 1, 1);
176+
CallOptions callOptions =
177+
CallOptions.DEFAULT
178+
.withOption(TRACER_KEY, throwingTracer)
179+
.withOption(REQUEST_ID_CALL_OPTIONS_KEY, requestId);
174180
MethodDescriptor<String, String> methodDescriptor = createMethodDescriptor();
175181
FakeChannel channel = new FakeChannel();
176182

177-
String requestId = "1.0000000000000001.1.1.1.1";
178-
RequestIdTargetTracker.record(requestId, "test-database", "endpoint-1", 100L, false);
183+
RequestIdTargetTracker.record(
184+
requestId.getLogicalRequestKey(), "test-database", "endpoint-1", 100L, false);
179185
assertNotNull(RequestIdTargetTracker.get(requestId));
180186

181187
ClientCall<String, String> call =
182188
interceptor.interceptCall(methodDescriptor, callOptions, channel);
183189
CapturingListener<String> responseListener = new CapturingListener<>();
184-
call.start(responseListener, createDefaultHeaders(requestId));
190+
call.start(responseListener, createDefaultHeaders(requestId.getHeaderValue()));
185191

186192
// Deliver onClose - even though metric recording throws, onClose must propagate downstream
187193
channel.lastListener.onClose(Status.OK, new Metadata());

0 commit comments

Comments
 (0)