yandrey321 opened a new pull request, #11425:
URL: https://github.com/apache/ozone/pull/11425
## What changes were proposed in this pull request?
Ozone forks Hadoop IPC under `org.apache.hadoop.ipc_` in
`hadoop-hdds/common`. `RpcMetrics` records
three per-call latency counters as `@Metric MutableRate` fields —
`rpcQueueTime`, `rpcLockWaitTime`,
`rpcProcessingTime` — and `Server` updates all three on *every* RPC from its
handler threads.
`MutableRate extends MutableStat`, and `MutableStat.add(long)` is
`synchronized(this)`, so with ~200 OM
handler threads every handler serialises on the same three monitors: a lock
convoy on the RPC serving
path. This is the same contention class already addressed for
`OMLockMetrics` / `PerformanceMetrics`
under HDDS-9377, but on the shared path fronting **OM, SCM and the
datanode**.
This converts those three counters to the existing lock-free
`org.apache.hadoop.ozone.util.ConcurrentMutableRate`, which accumulates into
`LongAdder` /
`LongAccumulator` and drains lazily on `snapshot()` / `lastStat()`.
### Why `RpcMetrics` has to own `getMetrics()`
`ConcurrentMutableRate` is not a `MutableRate` subclass, so the `@Metric`
field type cannot simply be
swapped: `MutableMetricsFactory.newForField` builds a real `MutableRate` for
a `MutableRate`-typed
field. The obvious alternative — construct the rate and register it — is
unavailable too, because
`MetricsRegistry.add(String, MutableMetric)` is package-private and
therefore unreachable from
`org.apache.hadoop.ipc_.metrics`. The three rates must be snapshotted by
hand.
Rather than hand-rolling the whole source on the `OMLockMetrics` template,
this uses the *hybrid* mode
`MetricsSourceBuilder.build()` already supports:
```java
if (source instanceof MetricsSource) {
if (hasAtMetric && !hasRegistry) throw new MetricsException("Hybrid
metrics: registry required.");
return (MetricsSource) source;
}
```
By the time `build()` runs, the builder's constructor has already reflected
every `@Metric` field and
method into the discovered registry and applied `registry.setContext("rpc")`
from `@Metrics`. So
`RpcMetrics` implements `MetricsSource`, keeps all ten `MutableCounterLong`
fields, the four `@Metric`
accessor methods, the registry, its `port`/`serverName` tags and the
`MutableQuantiles` arrays exactly
as they are, and only adds the three rates on top of the registry snapshot.
That matters for correctness, not just for diff size: one of the four
accessors,
`numOpenConnectionsPerUser()`, returns `String` and is emitted by metrics2
as a **tag**, not as a gauge.
A full rewrite would have had to reproduce that by hand; the hybrid route
never touches it.
### Metric names are unchanged
`MutableMetricsFactory.getName(Field)` is
`StringUtils.capitalize(field.getName())`, and
`@Metric.always()` defaults to `false`, so the fields emit
`RpcQueueTimeNumOps` / `RpcQueueTimeAvgTime`
and the two matching pairs, with no stddev/min/max. Constructing
`new ConcurrentMutableRate("RpcQueueTime", "Queue time", false)` reproduces
that exactly. Preserving
them is required: `ozone insight` reads `rpc_rpc_queue_time_avg_time` and
`rpc_rpc_processing_time_avg_time` in `BaseInsightPoint`, and operators'
dashboards and alerts key on
the same names.
Metric *ordering* within the record does change — the three rates move after
the registry-backed ones.
Sinks key by name, so this is immaterial.
### Scope
In scope: the three `MutableRate` fields.
Out of scope: `RpcDetailedMetrics`, whose single field is
`MutableRatesWithAggregation` and which
already accumulates per-thread and aggregates at snapshot (HADOOP-24420) to
avoid this exact lock, so it
is not a `ConcurrentMutableStat` drop-in; the `MutableCounterLong` counters,
which are already atomic;
and the `MutableQuantiles` arrays.
No wire or RPC protocol change, no configuration change, no new dependency.
One documented `ConcurrentMutableStat` caveat applies: a batched drain loses
within-batch variance, so
stddev is slightly underestimated under concurrency. The only in-tree
consumer is
`Server.logSlowRpcCalls`, a three-sigma coarse outlier heuristic gated on a
1024-sample minimum, for
which this is acceptable.
Generated-by: Claude Code (claude-opus)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16304
## How was this patch tested?
New unit test
`hadoop-hdds/common/src/test/java/org/apache/hadoop/ipc_/metrics/TestRpcMetrics.java`,
in
the same package so the package-private constructor is reachable without
registering into
`DefaultMetricsSystem`. It uses a mocked `Server` and reads the real emitted
record through
`MetricsCollectorImpl` rather than a mocked builder, pinning: the hybrid
wiring
(`MetricsAnnotations.makeSource(metrics)` returns the instance itself), the
six emitted rate names and
their values, the registry-backed metrics and the `port` / `serverName` /
`Context` /
`NumOpenConnectionsPerUser` tags all landing in the same `rpc` record, the
`getProcessingSampleCount/Mean/StdDev` values that feed slow-call detection,
and an exact sample count
under 14 threads × 1000 concurrent adds.
Name parity was then proven rather than argued: with the test in place,
`RpcMetrics.java` was reverted
to `git show HEAD:` and the suite re-run against the **pre-change** class.
Exactly one case fails — the
`assertSame` on the hybrid wiring, which is the one assertion about the new
behavior. The four cases
asserting metric names, values, the `rpc` record name and the tags pass
identically against both
versions.
- `mvn -pl :hdds-common test -Dtest=TestRpcMetrics` — 5/5.
- Full `hdds-common` unit suite — 569 tests, 0 failures, 0 errors.
- `mvn -pl
:ozone-manager,:hdds-server-scm,:hdds-container-service,:hdds-server-framework
-am clean
install -DskipTests` — BUILD SUCCESS. This is the blast-radius check the
Jira calls for: the `ipc_`
fork fronts OM, SCM and the datanode, and the `getRpcProcessingTime()`
return type changed.
- `mvn -pl :hdds-server-framework test
-Dtest=TestPrometheusMetricsIntegration` — 4/4, confirming the
reordered record still renders.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
