Skip to content

Commit 6f48225

Browse files
Addressed comments
1 parent f219b73 commit 6f48225

File tree

5 files changed

+37
-27
lines changed

5 files changed

+37
-27
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import com.google.monitoring.v3.ProjectName;
3636
import com.google.monitoring.v3.TimeSeries;
3737
import com.google.protobuf.Empty;
38-
import io.grpc.inprocess.InProcessChannelBuilder;
38+
import io.grpc.ManagedChannelBuilder;
3939
import io.opentelemetry.sdk.common.CompletableResultCode;
4040
import io.opentelemetry.sdk.metrics.InstrumentType;
4141
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
@@ -94,12 +94,15 @@ static SpannerCloudMonitoringExporter create(
9494
settingsBuilder.setUniverseDomain(universeDomain);
9595
}
9696

97-
if (System.getProperty("jmh.monitoring-server") != null) {
97+
if (System.getProperty("jmh.monitoring-server-port") != null) {
9898
settingsBuilder.setTransportChannelProvider(
9999
InstantiatingGrpcChannelProvider.newBuilder()
100100
.setChannelConfigurator(
101101
managedChannelBuilder ->
102-
InProcessChannelBuilder.forName(System.getProperty("jmh.monitoring-server")))
102+
ManagedChannelBuilder.forAddress(
103+
"0.0.0.0",
104+
Integer.parseInt(System.getProperty("jmh.monitoring-server-port")))
105+
.usePlaintext())
103106
.build());
104107
}
105108

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/BenchmarkValidator.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ void validate() {
5555
}
5656
Map<String, Double> actualPercentilesMap = actualResult.primaryMetric.scorePercentiles;
5757
// We will only be comparing the percentiles(p50, p90, p90) which are configured in the
58-
// expected
59-
// percentiles. This allows some checks to be disabled if required.
58+
// expected percentiles. This allows some checks to be disabled if required.
6059
for (Percentile expectedPercentile : expectResult.scorePercentiles) {
6160
String percentile = expectedPercentile.percentile;
6261
double difference =
@@ -135,19 +134,23 @@ static class ValidationException extends RuntimeException {
135134
}
136135
}
137136

138-
private static String parseCommandLineArg(String arg) {
139-
if (arg == null || arg.isEmpty()) {
137+
private static String parseCommandLineArgs(String[] args, String key) {
138+
if (args == null) {
140139
return "";
141140
}
142-
String[] args = arg.split("=");
143-
if (args.length != 2) {
144-
return "";
141+
for (String arg : args) {
142+
if (arg.startsWith("--" + key)) {
143+
String[] splits = arg.split("=");
144+
if (splits.length == 2) {
145+
return splits[1].trim();
146+
}
147+
}
145148
}
146-
return args[1];
149+
return "";
147150
}
148151

149152
public static void main(String[] args) {
150-
String actualFile = parseCommandLineArg(args[0]);
153+
String actualFile = parseCommandLineArgs(args, "file");
151154
new BenchmarkValidator("com/google/cloud/spanner/jmh/jmh-baseline.json", actualFile).validate();
152155
}
153156
}

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/MonitoringServiceImpl.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.monitoring.v3.CreateTimeSeriesRequest;
2020
import com.google.monitoring.v3.MetricServiceGrpc.MetricServiceImplBase;
2121
import com.google.protobuf.Empty;
22+
import io.grpc.Status;
2223
import io.grpc.stub.StreamObserver;
2324

2425
class MonitoringServiceImpl extends MetricServiceImplBase {
@@ -31,7 +32,8 @@ public void createServiceTimeSeries(
3132
responseObserver.onNext(Empty.getDefaultInstance());
3233
responseObserver.onCompleted();
3334
} catch (InterruptedException e) {
34-
responseObserver.onError(e);
35+
responseObserver.onError(
36+
Status.CANCELLED.withCause(e).withDescription(e.getMessage()).asException());
3537
}
3638
}
3739
}

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/ReadBenchmark.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@
3232
import com.google.spanner.v1.StructType;
3333
import com.google.spanner.v1.StructType.Field;
3434
import com.google.spanner.v1.TypeCode;
35+
import io.grpc.ManagedChannelBuilder;
3536
import io.grpc.Server;
36-
import io.grpc.inprocess.InProcessChannelBuilder;
37-
import io.grpc.inprocess.InProcessServerBuilder;
37+
import io.grpc.ServerBuilder;
3838
import java.io.IOException;
3939
import java.util.Arrays;
4040
import java.util.List;
@@ -99,9 +99,8 @@ public void setup() throws IOException {
9999
gRPCServerExecutor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
100100

101101
// Creating Spanner Inprocess gRPC server
102-
String serverName = InProcessServerBuilder.generateName();
103102
gRPCServer =
104-
InProcessServerBuilder.forName(serverName)
103+
ServerBuilder.forPort(0)
105104
.addService(mockSpannerService)
106105
.executor(gRPCServerExecutor)
107106
.build()
@@ -110,21 +109,20 @@ public void setup() throws IOException {
110109
registerMocks(mockSpannerService);
111110

112111
// Creating Monitoring Inprocess gRPC server
113-
String monitoringServerName = InProcessServerBuilder.generateName();
114112
gRPCMonitoringServer =
115-
InProcessServerBuilder.forName(monitoringServerName)
116-
.addService(mockMonitoringService)
117-
.build()
118-
.start();
113+
ServerBuilder.forPort(0).addService(mockMonitoringService).build().start();
119114

120-
// Set the monitoring host for exporter to forward requests to inprocess gRPC server
121-
System.setProperty("jmh.monitoring-server", monitoringServerName);
115+
// Set the monitoring host port for exporter to forward requests to local netty gRPC server
116+
System.setProperty(
117+
"jmh.monitoring-server-port", String.valueOf(gRPCMonitoringServer.getPort()));
122118

123119
spanner =
124120
SpannerOptions.newBuilder()
125121
.setProjectId("[PROJECT]")
126122
.setChannelConfigurator(
127-
managedChannelBuilder -> InProcessChannelBuilder.forName(serverName))
123+
managedChannelBuilder ->
124+
ManagedChannelBuilder.forAddress("0.0.0.0", gRPCServer.getPort())
125+
.usePlaintext())
128126
.build()
129127
.getService();
130128
databaseClient =
@@ -172,10 +170,14 @@ private void registerMocks(MockSpannerServiceImpl mockSpannerService) {
172170
}
173171

174172
@TearDown(Level.Trial)
175-
public void tearDown() {
173+
public void tearDown() throws InterruptedException {
176174
spanner.close();
177175
gRPCServer.shutdown();
178176
gRPCServerExecutor.shutdown();
177+
178+
// awaiting termination for servers and executors
179+
gRPCServer.awaitTermination(10, TimeUnit.SECONDS);
180+
gRPCServerExecutor.awaitTermination(10, TimeUnit.SECONDS);
179181
}
180182
}
181183

google-cloud-spanner/src/test/resources/com/google/cloud/spanner/jmh/jmh-baseline.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"baselineConfigurations": {
2+
"benchmarkResultMap": {
33
"com.google.cloud.spanner.benchmarking.ReadBenchmark.queryBenchmark": {
44
"scorePercentiles": [
55
{

0 commit comments

Comments
 (0)