[
https://issues.apache.org/jira/browse/HADOOP-13782?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15645199#comment-15645199
]
Zhe Zhang commented on HADOOP-13782:
------------------------------------
Thanks Erik for the patch. LGTM overall. A few detailed comments:
# It'd be ideal if we can simplify the two internal classes
{{LocalMutableRate}} and {{MutableRateInternal}}, and also better fit them with
existing {{MutableStat}} or {{MutableRate}} classes. We discussed offline an
issue in existing {{MutableStat}} batch add method around {{intervalStat}}. I
think we should document the issue so other developers understand the
motivation of creating a simpler rate class.
# The below synchronization behavior is different than {{MutableStat}}, where
both {{snapshot}} and {{add}} methods are {{synchronized}}. Should we allow
thread-local {{add}} while one thread is doing {{snapshot}}?
{code}
@Override
public void snapshot(MetricsRecordBuilder rb, boolean all) {
synchronized (globalMetrics) {
{code}
# Maybe we should comment below that we will be doing aggregation (a main logic
in this class)
{code}
} else {
for (Map.Entry<String, LocalMutableRate> ent : map.entrySet()) {
{code}
# Cosmetic: since {{getLocalMetrics}} is short and is only used by {{add}}
(which itself is short), can we merge the two methods?
# Cosmetic: as a follow-on we can consider consolidating the old
{{MutableRates}} and the new {{MutableRatesWithAggregation}} to reduce
duplication
> Make MutableRates metrics thread-local write, aggregate-on-read
> ---------------------------------------------------------------
>
> Key: HADOOP-13782
> URL: https://issues.apache.org/jira/browse/HADOOP-13782
> Project: Hadoop Common
> Issue Type: Improvement
> Components: metrics
> Reporter: Erik Krogen
> Assignee: Erik Krogen
> Attachments: HADOOP-13782.000.patch, HADOOP-13782.001.patch
>
>
> Currently the {{MutableRates}} metrics class serializes all writes to metrics
> it contains because of its use of {{MetricsRegistry.add()}} (i.e., even two
> increments of unrelated metrics contained within the same {{MutableRates}}
> object will serialize w.r.t. each other). This class is used by
> {{RpcDetailedMetrics}}, which may have many hundreds of threads contending to
> modify these metrics. Instead we should allow updates to unrelated metrics
> objects to happen concurrently. To do so we can let each thread locally
> collect metrics, and on a {{snapshot}}, aggregate the metrics from all of the
> threads.
> I have collected some benchmark performance numbers in HADOOP-13747
> (https://issues.apache.org/jira/secure/attachment/12835043/benchmark_results)
> which indicate that this can bring significantly higher performance in high
> contention situations.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
