[
https://issues.apache.org/jira/browse/HDDS-16304?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Wei-Chiu Chuang updated HDDS-16304:
-----------------------------------
Fix Version/s: 2.3.0
Resolution: Fixed
Status: Resolved (was: Patch Available)
> 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
> Priority: Major
> Labels: pull-request-available
> Fix For: 2.3.0
>
>
> *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]
