[
https://issues.apache.org/jira/browse/HDDS-16740?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey Yarovoy reassigned HDDS-16740:
-------------------------------------
Assignee: Andrey Yarovoy
> Convert GrpcMetrics latency counters to lock-free ConcurrentMutableRate
> -----------------------------------------------------------------------
>
> Key: HDDS-16740
> URL: https://issues.apache.org/jira/browse/HDDS-16740
> Project: Apache Ozone
> Issue Type: Improvement
> Components: Ozone Manager
> Reporter: Andrey Yarovoy
> Assignee: Andrey Yarovoy
> Priority: Major
>
> `GrpcMetrics` keeps its two per-call latency counters as `@Metric
> MutableRate` fields —
> `grpcQueueTime` and `grpcProcessingTime` in
> `hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/grpc/metrics/GrpcMetrics.java`.
> Both are
> updated from `GrpcMetricsServerRequestInterceptor.onComplete()`, i.e. on
> every gRPC call served by
> `GrpcOzoneManagerServer`:
> {code:java}
> grpcMetrics.addGrpcQueueTime(queueTime);
> grpcMetrics.addGrpcProcessingTime(processingTime);
> {code}
> `MutableRate extends MutableStat`, and `MutableStat.add(long)` is
> `synchronized(this)`, so every gRPC
> server thread serialises on the same two monitors. This is the same
> contention class as HDDS-16304
> (`RpcMetrics` on the Hadoop IPC fork) and HDDS-9377 (`OMLockMetrics` /
> `PerformanceMetrics`), on the
> path S3 Gateway uses to reach OM. The interceptors are installed
> unconditionally in
> `GrpcOzoneManagerServer`, and `ozone.om.s3.grpc.server_enabled` defaults to
> `true`, so this path is
> live on a default cluster.
> Replace both with the existing lock-free
> `org.apache.hadoop.ozone.util.ConcurrentMutableRate`, which
> accumulates into `LongAdder` / `LongAccumulator` and drains lazily on
> `snapshot()` / `lastStat()`.
>
> Why this is smaller than HDDS-16304
> `GrpcMetrics` already implements `MetricsSource` and already owns
> `getMetrics()`, so the hybrid-source
> plumbing HDDS-16304 had to introduce is here for free. The change is:
> - drop `@Metric` from the two fields and make them `final
> ConcurrentMutableRate`, constructed in the
> constructor;
> - add two `snapshot(builder, all)` calls to the existing `getMetrics()`,
> next to the current
> `registry.snapshot(...)`;
> - change the two package-private getters' return type (they have no callers
> in the tree).
> Everything else — the five `MutableCounterLong` fields, the registry, the
> `MutableQuantiles` arrays and
> their `registry.newQuantiles(...)` creation, the second record carrying the
> `LatestRequestType` tag — is
> untouched.
>
> *Emitted names must not change*
> `MutableMetricsFactory.getName(Field)` is
> `StringUtils.capitalize(field.getName())` and
> `@Metric.always()` defaults to `false`, so the fields currently emit
> `GrpcQueueTimeNumOps` /
> `GrpcQueueTimeAvgTime` and `GrpcProcessingTimeNumOps` /
> `GrpcProcessingTimeAvgTime`, with no
> stddev/min/max. `new ConcurrentMutableRate("GrpcQueueTime", "Queue time",
> false)` and
> `("GrpcProcessingTime", "Processing time", false)` reproduce that exactly.
> There are no in-tree
> consumers of these names (unlike the `RpcMetrics` ones, which `ozone insight`
> reads), but operator
> dashboards key on them, so they stay byte-identical.
> Documented `ConcurrentMutableStat` caveat: a batched drain loses within-batch
> variance, so stddev is
> slightly underestimated under concurrency. Nothing in the tree reads the
> stddev of these two rates.
>
> *Scope*
> In scope: the two `MutableRate` fields in `GrpcMetrics`.
> Out of scope: the `MutableCounterLong` counters (already atomic) and the
> `MutableQuantiles` arrays.
> No wire or RPC protocol change, no configuration change, no new dependency.
>
> *Testing*
> Unit test in `hadoop-hdds/framework`, following
> `hadoop-hdds/common/src/test/java/org/apache/hadoop/ipc_/metrics/TestRpcMetrics.java`
> from HDDS-16304:
> read the real emitted record through `MetricsCollectorImpl` and pin the four
> rate names and their
> values, that the registry-backed counters and the `LatestRequestType` tag
> still appear, and an exact
> sample count under concurrent `addGrpcProcessingTime`. Name parity can be
> proven the same way it was
> for HDDS-16304 — revert the main file to `git show HEAD:` with the new test
> in place and confirm only
> the assertions about the new wiring fail.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
