hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zhe Zhang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-13782) Make MutableRates metrics thread-local write, aggregate-on-read
Date Mon, 07 Nov 2016 19:39:58 GMT

    [ 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: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message