From 6119f6ec945fdbdb3015d1047f67def8d5ff136b Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 2 Feb 2023 15:16:27 -0800 Subject: [PATCH] services: add qps in orca api (#9866) --- repositories.bzl | 6 ++--- .../io/grpc/services/CallMetricRecorder.java | 22 +++++++++++++++++-- .../services/InternalCallMetricRecorder.java | 6 ++--- .../java/io/grpc/services/MetricRecorder.java | 17 +++++++++++++- .../java/io/grpc/services/MetricReport.java | 13 ++++++++--- .../grpc/services/CallMetricRecorderTest.java | 5 +++++ .../io/grpc/xds/orca/OrcaPerRequestUtil.java | 4 ++-- .../io/grpc/xds/orca/OrcaServiceImpl.java | 1 + .../io/grpc/xds/orca/OrcaServiceImplTest.java | 9 +++++++- 9 files changed, 68 insertions(+), 15 deletions(-) diff --git a/repositories.bzl b/repositories.bzl index cced1d29be..c7c9cf736c 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -95,10 +95,10 @@ def grpc_java_repositories(): if not native.existing_rule("com_github_cncf_xds"): http_archive( name = "com_github_cncf_xds", - strip_prefix = "xds-d92e9ce0af512a73a3a126b32fa4920bee12e180", - sha256 = "27be88b1ff2844885d3b2d0d579546f3a8b3f26b4871eed89082c9709e49a4bd", + strip_prefix = "xds-06c439db220b89134a8a49bad41994560d6537c6", + sha256 = "41ea212940ab44bf7f8a8b4169cfbc612ed2166dafabc0a56a8820ef665fc6a4", urls = [ - "https://github.com/cncf/xds/archive/d92e9ce0af512a73a3a126b32fa4920bee12e180.tar.gz", + "https://github.com/cncf/xds/archive/06c439db220b89134a8a49bad41994560d6537c6.tar.gz", ], ) if not native.existing_rule("com_github_grpc_grpc"): diff --git a/services/src/main/java/io/grpc/services/CallMetricRecorder.java b/services/src/main/java/io/grpc/services/CallMetricRecorder.java index d93f93606f..8aff9a0d8a 100644 --- a/services/src/main/java/io/grpc/services/CallMetricRecorder.java +++ b/services/src/main/java/io/grpc/services/CallMetricRecorder.java @@ -43,6 +43,7 @@ public final class CallMetricRecorder { new AtomicReference<>(); private double cpuUtilizationMetric = 0; private double memoryUtilizationMetric = 0; + private double qps = 0; private volatile boolean disabled; /** @@ -158,6 +159,23 @@ public final class CallMetricRecorder { return this; } + /** + * Records a call metric measurement for qps. + * If RPC has already finished, this method is no-op. + * + *

A latter record will overwrite its former name-sakes. + * + * @return this recorder object + * @since 1.54.0 + */ + public CallMetricRecorder recordQpsMetric(double value) { + if (disabled) { + return this; + } + qps = value; + return this; + } + /** * Returns all request cost metric values. No more metric values will be recorded after this @@ -187,8 +205,8 @@ public final class CallMetricRecorder { if (savedUtilizationMetrics == null) { savedUtilizationMetrics = Collections.emptyMap(); } - return new MetricReport(cpuUtilizationMetric, - memoryUtilizationMetric, Collections.unmodifiableMap(savedRequestCostMetrics), + return new MetricReport(cpuUtilizationMetric, memoryUtilizationMetric, qps, + Collections.unmodifiableMap(savedRequestCostMetrics), Collections.unmodifiableMap(savedUtilizationMetrics) ); } diff --git a/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java b/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java index 97e5e5a0aa..6cee9048c4 100644 --- a/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java +++ b/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java @@ -46,8 +46,8 @@ public final class InternalCallMetricRecorder { } public static MetricReport createMetricReport(double cpuUtilization, double memoryUtilization, - Map requestCostMetrics, Map utilizationMetrics) { - return new MetricReport(cpuUtilization, memoryUtilization, - requestCostMetrics, utilizationMetrics); + double qps, Map requestCostMetrics, Map utilizationMetrics) { + return new MetricReport(cpuUtilization, memoryUtilization, qps, requestCostMetrics, + utilizationMetrics); } } diff --git a/services/src/main/java/io/grpc/services/MetricRecorder.java b/services/src/main/java/io/grpc/services/MetricRecorder.java index a576386e98..7a541cceea 100644 --- a/services/src/main/java/io/grpc/services/MetricRecorder.java +++ b/services/src/main/java/io/grpc/services/MetricRecorder.java @@ -30,6 +30,7 @@ public final class MetricRecorder { private volatile ConcurrentHashMap metricsData = new ConcurrentHashMap<>(); private volatile double cpuUtilization; private volatile double memoryUtilization; + private volatile double qps; public static MetricRecorder newInstance() { return new MetricRecorder(); @@ -86,8 +87,22 @@ public final class MetricRecorder { memoryUtilization = 0; } + /** + * Update the QPS metrics data. + */ + public void setQps(double value) { + qps = value; + } + + /** + * Clear the QPS metrics data. + */ + public void clearQps() { + qps = 0; + } + MetricReport getMetricReport() { - return new MetricReport(cpuUtilization, memoryUtilization, + return new MetricReport(cpuUtilization, memoryUtilization, qps, Collections.emptyMap(), Collections.unmodifiableMap(metricsData)); } } diff --git a/services/src/main/java/io/grpc/services/MetricReport.java b/services/src/main/java/io/grpc/services/MetricReport.java index 56ab150f8a..73aba7a2af 100644 --- a/services/src/main/java/io/grpc/services/MetricReport.java +++ b/services/src/main/java/io/grpc/services/MetricReport.java @@ -30,14 +30,16 @@ import java.util.Map; public final class MetricReport { private double cpuUtilization; private double memoryUtilization; + private double qps; private Map requestCostMetrics; private Map utilizationMetrics; - MetricReport(double cpuUtilization, double memoryUtilization, - Map requestCostMetrics, - Map utilizationMetrics) { + MetricReport(double cpuUtilization, double memoryUtilization, double qps, + Map requestCostMetrics, + Map utilizationMetrics) { this.cpuUtilization = cpuUtilization; this.memoryUtilization = memoryUtilization; + this.qps = qps; this.requestCostMetrics = checkNotNull(requestCostMetrics, "requestCostMetrics"); this.utilizationMetrics = checkNotNull(utilizationMetrics, "utilizationMetrics"); } @@ -58,6 +60,10 @@ public final class MetricReport { return utilizationMetrics; } + public double getQps() { + return qps; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -65,6 +71,7 @@ public final class MetricReport { .add("memoryUtilization", memoryUtilization) .add("requestCost", requestCostMetrics) .add("utilization", utilizationMetrics) + .add("qps", qps) .toString(); } } diff --git a/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java b/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java index 9811d1da92..e4f4155c9b 100644 --- a/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java +++ b/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java @@ -46,6 +46,7 @@ public class CallMetricRecorderTest { recorder.recordRequestCostMetric("cost3", 1.0); recorder.recordCpuUtilizationMetric(0.1928); recorder.recordMemoryUtilizationMetric(47.4); + recorder.recordQpsMetric(2522.54); MetricReport dump = recorder.finalizeAndDump2(); Truth.assertThat(dump.getUtilizationMetrics()) @@ -54,6 +55,7 @@ public class CallMetricRecorderTest { .containsExactly("cost1", 37465.12, "cost2", 10293.0, "cost3", 1.0); Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0.1928); Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(47.4); + Truth.assertThat(dump.getQps()).isEqualTo(2522.54); } @Test @@ -75,6 +77,8 @@ public class CallMetricRecorderTest { recorder.recordUtilizationMetric("util1", 28374.21); recorder.recordMemoryUtilizationMetric(9384.0); recorder.recordUtilizationMetric("util1", 84323.3); + recorder.recordQpsMetric(1928.3); + recorder.recordQpsMetric(100.8); MetricReport dump = recorder.finalizeAndDump2(); Truth.assertThat(dump.getRequestCostMetrics()) @@ -83,6 +87,7 @@ public class CallMetricRecorderTest { Truth.assertThat(dump.getUtilizationMetrics()) .containsExactly("util1", 84323.3); Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0); + Truth.assertThat(dump.getQps()).isEqualTo(100.8); } @Test diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java b/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java index 0c2c7395b4..9741452967 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java @@ -254,8 +254,8 @@ public abstract class OrcaPerRequestUtil { static MetricReport fromOrcaLoadReport(OrcaLoadReport loadReport) { return InternalCallMetricRecorder.createMetricReport(loadReport.getCpuUtilization(), - loadReport.getMemUtilization(), loadReport.getRequestCostMap(), - loadReport.getUtilizationMap()); + loadReport.getMemUtilization(), loadReport.getRpsFractional(), + loadReport.getRequestCostMap(), loadReport.getUtilizationMap()); } /** diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java b/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java index 1ea64f70bf..30522a5e0f 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java @@ -150,6 +150,7 @@ public final class OrcaServiceImpl implements BindableService { InternalMetricRecorder.getMetricReport(metricRecorder); return OrcaLoadReport.newBuilder().setCpuUtilization(internalReport.getCpuUtilization()) .setMemUtilization(internalReport.getMemoryUtilization()) + .setRpsFractional(internalReport.getQps()) .putAllUtilization(internalReport.getUtilizationMetrics()) .build(); } diff --git a/xds/src/test/java/io/grpc/xds/orca/OrcaServiceImplTest.java b/xds/src/test/java/io/grpc/xds/orca/OrcaServiceImplTest.java index a292df0f03..1063b2bec0 100644 --- a/xds/src/test/java/io/grpc/xds/orca/OrcaServiceImplTest.java +++ b/xds/src/test/java/io/grpc/xds/orca/OrcaServiceImplTest.java @@ -143,19 +143,23 @@ public class OrcaServiceImplTest { ClientCall call = channel.newCall( OpenRcaServiceGrpc.getStreamCoreMetricsMethod(), CallOptions.DEFAULT); defaultTestService.putUtilizationMetric("buffer", 0.2); + defaultTestService.setQps(1.9); call.start(listener, new Metadata()); call.sendMessage(OrcaLoadReportRequest.newBuilder() .setReportInterval(Duration.newBuilder().setSeconds(0).setNanos(500).build()).build()); call.halfClose(); call.request(1); - OrcaLoadReport expect = OrcaLoadReport.newBuilder().putUtilization("buffer", 0.2).build(); + OrcaLoadReport expect = OrcaLoadReport.newBuilder().putUtilization("buffer", 0.2) + .setRpsFractional(1.9).build(); verify(listener).onMessage(eq(expect)); reset(listener); defaultTestService.removeUtilizationMetric("buffer0"); + defaultTestService.clearQps(); assertThat(fakeClock.forwardTime(500, TimeUnit.NANOSECONDS)).isEqualTo(0); verifyNoInteractions(listener); assertThat(fakeClock.forwardTime(1, TimeUnit.SECONDS)).isEqualTo(1); call.request(1); + expect = OrcaLoadReport.newBuilder().putUtilization("buffer", 0.2).build(); verify(listener).onMessage(eq(expect)); } @@ -245,17 +249,20 @@ public class OrcaServiceImplTest { .setMemUtilization(random.nextDouble()) .putAllUtilization(firstUtilization) .putUtilization("queue", 1.0) + .setRpsFractional(1239.01) .build(); defaultTestService.setCpuUtilizationMetric(goldenReport.getCpuUtilization()); defaultTestService.setMemoryUtilizationMetric(goldenReport.getMemUtilization()); defaultTestService.setAllUtilizationMetrics(firstUtilization); defaultTestService.putUtilizationMetric("queue", 1.0); + defaultTestService.setQps(1239.01); Iterator reports = OpenRcaServiceGrpc.newBlockingStub(channel) .streamCoreMetrics(OrcaLoadReportRequest.newBuilder().build()); assertThat(reports.next()).isEqualTo(goldenReport); defaultTestService.clearCpuUtilizationMetric(); defaultTestService.clearMemoryUtilizationMetric(); + defaultTestService.clearQps(); fakeClock.forwardTime(1, TimeUnit.SECONDS); goldenReport = OrcaLoadReport.newBuilder() .putAllUtilization(firstUtilization)