HBase
  1. HBase
  2. HBASE-10129

support real time rpc invoke latency percentile statistics for methods of HRegionInterface

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.94.14
    • Fix Version/s: None
    • Component/s: regionserver
    • Labels:
      None

      Description

      It is important for applications to get latency statistics when invoking hbase apis. Currently, the average latency of methods in HRegionInterface will be computed in HBaseRpcMetrics. However, user might expect more detail latency statistics, such as 75% percentile latency, 95% percentile latency, etc. Therefore, will it be useful if we computing latency percentiles for rpc invoking of region server methods?

        Activity

        Hide
        cuijianwei added a comment -

        To evaluate the performance change, I use the test tool YCSB to put/get 10,000,000 records to a cluster containing 5 region servers, then I compare the average latency respectively.
        For cluster without the patch, I get the put/get latency:
        [PUT], AverageLatency(us), 953.5947375
        [GET], AverageLatency(us), 279.0727035
        For cluster with the patch, I get the put/get latency:
        [PUT], AverageLatency(us), 954.3327765
        [GET], AverageLatency(us), 248.4747838
        It seems the performance change when applying the patch is not significant.

        Show
        cuijianwei added a comment - To evaluate the performance change, I use the test tool YCSB to put/get 10,000,000 records to a cluster containing 5 region servers, then I compare the average latency respectively. For cluster without the patch, I get the put/get latency: [PUT] , AverageLatency(us), 953.5947375 [GET] , AverageLatency(us), 279.0727035 For cluster with the patch, I get the put/get latency: [PUT] , AverageLatency(us), 954.3327765 [GET] , AverageLatency(us), 248.4747838 It seems the performance change when applying the patch is not significant.
        Hide
        cuijianwei added a comment -

        Nick Dimiduk, thanks, I will try the it.

        Show
        cuijianwei added a comment - Nick Dimiduk , thanks, I will try the it.
        Hide
        Nick Dimiduk added a comment -

        Is there any benchmark when doing efficiency comparisons?

        Have a look at org.apache.hadoop.hbase.PerformanceEvaluation.

        Show
        Nick Dimiduk added a comment - Is there any benchmark when doing efficiency comparisons? Have a look at org.apache.hadoop.hbase.PerformanceEvaluation .
        Hide
        cuijianwei added a comment -

        Elliott Clark, I agree this feature should not affect hbase performance. I think we might add a configuration option such as 'regionserver.rpc.latency.percentile.enable' to enable or disable this feature. Then, users can decide whether enable this feature according to their needs. We need to do some comparisons to efficiency change after enable this feature. Is there any benchmark when doing efficiency comparisons?

        Show
        cuijianwei added a comment - Elliott Clark , I agree this feature should not affect hbase performance. I think we might add a configuration option such as 'regionserver.rpc.latency.percentile.enable' to enable or disable this feature. Then, users can decide whether enable this feature according to their needs. We need to do some comparisons to efficiency change after enable this feature. Is there any benchmark when doing efficiency comparisons?
        Hide
        cuijianwei added a comment -

        Thanks for your comment Nick Dimiduk and Elliott Clark. I go through the metric implementation of trunk, the latency percentile of many methods in HRegionInterface has been computed. Therefore, this patch might aim to help 0.94 to contain this feature? The implementation of 'MetricsTimeVaryingHistogram.registerTimeVaryingHistogramMetric()' will register a metrics for the first time and do updating after register. I think it might be more clear if we rename 'registerTimeVaryingHistogramMetric()' to 'registerOrUpdateTimeVaryingHistogramMetric()'.

        Show
        cuijianwei added a comment - Thanks for your comment Nick Dimiduk and Elliott Clark . I go through the metric implementation of trunk, the latency percentile of many methods in HRegionInterface has been computed. Therefore, this patch might aim to help 0.94 to contain this feature? The implementation of 'MetricsTimeVaryingHistogram.registerTimeVaryingHistogramMetric()' will register a metrics for the first time and do updating after register. I think it might be more clear if we rename 'registerTimeVaryingHistogramMetric()' to 'registerOrUpdateTimeVaryingHistogramMetric()'.
        Hide
        Elliott Clark added a comment -

        Most of this is already done in trunk (The most important HRegion have metrics). I would be wary of this. The old 0.94 metrics system is pretty prone to slow downs when adding metrics, and it looks like these will be hit a lot. How much does this change the performance of HBase ?

        Show
        Elliott Clark added a comment - Most of this is already done in trunk (The most important HRegion have metrics). I would be wary of this. The old 0.94 metrics system is pretty prone to slow downs when adding metrics, and it looks like these will be hit a lot. How much does this change the performance of HBase ?
        Hide
        Nick Dimiduk added a comment -

        Nice feature. Any chance of a patch for trunk?

        Skimmed the patch:

           public void doUpdates(final MetricsContext context) {
        +    for (Entry<String, MetricsTimeVaryingHistogram> entry : methodHistograms.entrySet()) {
        +      MetricsTimeVaryingHistogram.registerTimeVaryingHistogramMetric(entry.getKey(),
        +        entry.getValue(), this.registry);
        +      entry.getValue().pushMetric(metricsRecord);
        +    }
        

        Can registration be handled once on initialization rather than with every call do doUpdates?

        Show
        Nick Dimiduk added a comment - Nice feature. Any chance of a patch for trunk? Skimmed the patch: public void doUpdates(final MetricsContext context) { + for (Entry<String, MetricsTimeVaryingHistogram> entry : methodHistograms.entrySet()) { + MetricsTimeVaryingHistogram.registerTimeVaryingHistogramMetric(entry.getKey(), + entry.getValue(), this.registry); + entry.getValue().pushMetric(metricsRecord); + } Can registration be handled once on initialization rather than with every call do doUpdates?
        cuijianwei made changes -
        Field Original Value New Value
        Attachment HBASE-10129-0.94-v1.patch [ 12618229 ]
        Hide
        cuijianwei added a comment -

        This patch will computing real time rpc invoke latency percentiles for methods of HRegionInterface, including:
        1. Based on MetricsHistogram, implement MetricsTimeVaryingHistogram which will compute real time percentiles for metric;
        2. Add 999th percentile statistics for MetricsHistogram;
        3. Extend HBaseRpcMetrics to compute rpc invoke real time latency percentiles based on MetricsTimeVaryingHistogram.

        Show
        cuijianwei added a comment - This patch will computing real time rpc invoke latency percentiles for methods of HRegionInterface, including: 1. Based on MetricsHistogram, implement MetricsTimeVaryingHistogram which will compute real time percentiles for metric; 2. Add 999th percentile statistics for MetricsHistogram; 3. Extend HBaseRpcMetrics to compute rpc invoke real time latency percentiles based on MetricsTimeVaryingHistogram.
        cuijianwei created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            cuijianwei
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:

              Development