Andrey Yarovoy created HDDS-16304:
-------------------------------------
Summary: Convert RpcMetrics latency counters to lock-free
ConcurrentMutableStat
Key: HDDS-16304
URL: https://issues.apache.org/jira/browse/HDDS-16304
Project: Apache Ozone
Issue Type: Improvement
Reporter: Andrey Yarovoy
Assignee: Andrey Yarovoy
*Problem*
Ozone forks the Hadoop IPC layer under {{org.apache.hadoop.ipc_}}
({{{}hadoop-hdds/common{}}}). {{RpcMetrics}} records three per-call latency
counters as Hadoop {{@Metric MutableRate}} fields:
* {{rpcQueueTime}}
* {{rpcLockWaitTime}}
* {{rpcProcessingTime}}
These are updated on *every RPC* via {{addRpcQueueTime}} /
{{addRpcLockWaitTime}} / {{{}addRpcProcessingTime{}}}, called from the IPC
{{Server}} handler threads. {{{}MutableRate extends MutableStat{}}}, and
{{MutableStat.add(long)}} is {{{}synchronized(this){}}}. With a large handler
pool (e.g. OM runs ~200 handlers), all threads recording a sample serialize on
the _same_ metric monitor — a lock convoy on the RPC serving path. This is the
same contention class already addressed for OM metrics in HDDS-9377
({{{}ConcurrentMutableStat{}}}) and for {{OMPerformanceMetrics}} on the current
branch, but on the shared IPC server path that fronts *OM, SCM, and datanode*
RPC servers.
*Approach*
Reuse the existing lock-free {{ConcurrentMutableStat}} (striped
{{{}LongAdder{}}}/{{{}LongAccumulator{}}}, drained lazily at snapshot)
introduced in HDDS-9377, via the {{ConcurrentMutableRate}} convenience subclass
(public ctor, {{sampleName="Ops"}} / {{{}valueName="Time"{}}}).
Because {{@Metric}} fields are instantiated reflectively by
{{MutableMetricsFactory}} (which always creates a real {{{}MutableRate{}}}),
the field type cannot simply be swapped. {{RpcMetrics}} must be reworked into a
hand-rolled {{MetricsSource}} (same template as {{OMPerformanceMetrics}} /
{{{}OMLockMetrics{}}}): construct the three rates in the constructor as
{{{}ConcurrentMutableRate{}}}, keep the {{{}MetricsRegistry{}}}, and implement
{{getMetrics()}} to snapshot the three rates plus the existing
{{MutableCounterLong}} counters, the {{@Metric}} gauge methods
({{{}numOpenConnections{}}}, {{{}callQueueLength{}}},
{{{}numDroppedConnections{}}}, {{{}numOpenConnectionsPerUser{}}}), and the
conditional {{MutableQuantiles[]}} arrays. The lock-free counters must keep the
*same emitted metric names*
({{{}RpcQueueTimeNumOps{}}}/{{{}RpcQueueTimeAvgTime{}}}, etc.) — guaranteed by
construction (capitalized name + {{{}extended=false{}}}).
{{Server.java:463-468}} consumes {{getProcessingMean()}} /
{{getProcessingStdDev()}} / {{{}getProcessingSampleCount(){}}}, which delegate
to {{{}rpcProcessingTime.lastStat(){}}}. {{ConcurrentMutableStat}} overrides
{{lastStat()}} to drain pending samples first, so these callers keep working;
the getter return type changes from {{MutableRate}} to
{{{}ConcurrentMutableStat{}}}/{{{}MutableStat{}}}.
*Scope / non-goals*
* *In scope:* the 3 {{MutableRate}} fields in {{RpcMetrics}} only.
* *Out of scope:* {{{}RpcDetailedMetrics{}}}. It uses
{{{}MutableRatesWithAggregation{}}}, *not* a plain {{{}MutableRate{}}}. That
class already accumulates per-thread and aggregates at snapshot (HADOOP-24420)
specifically to avoid this lock, so it is not a {{ConcurrentMutableStat}}
drop-in and needs no change here. Left as a separate follow-up only if
profiling later shows its aggregation step is itself hot.
* The {{MutableCounterLong}} counters ({{{}AtomicLong{}}}-backed) and
{{MutableQuantiles}} are not part of this change.
*Blast radius / compatibility*
Shared {{hadoop-hdds/common}} {{ipc_}} fork → touches OM, SCM, and datanode RPC
servers. No wire/RPC protocol change. All emitted metric names ({{{}rpc{}}}
context) are preserved byte-identically, so dashboards/alerts are unaffected.
Std-dev is slightly underestimated under concurrent batched adds (documented
{{ConcurrentMutableStat}} caveat) — acceptable for these latency stats and for
the {{Server}} three-sigma slow-call heuristic.
*Testing*
* Build {{hadoop-hdds/common}} + dependents (OM/SCM/datanode) to confirm the
{{{}getRpcProcessingTime{}}}/{{{}lastStat(){}}} callers in {{Server.java}}
compile against the new getter type.
* Unit test asserting the three metric names still appear with correct
{{{}NumOps{}}}/{{{}AvgTime{}}} after {{add()}} + snapshot.
* Sanity-check {{{}Server{}}}'s slow-call detection path still reads a sane
mean/stddev/sample-count after conversion.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
