HDDS-10862. Reduce time spent on initializing metrics during OM start #6682

mango-li · 2024-05-15T12:06:26Z

What changes were proposed in this pull request?

In the previous code, the metric was updated synchronously when OM started. There should be changed to asynchronous to reduce time consumption.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10862

How was this patch tested?

exist UT.

adoroszlai

Thanks @mango-li for the patch.

adoroszlai · 2024-05-15T12:19:36Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

+        if (getMetricsStorageFile().exists()) {
+          OmMetricsInfo metricsInfo = READER.readValue(getMetricsStorageFile());
+          metrics.setNumKeys(metricsInfo.getNumKeys());
+        }


This sets metrics.numKeys from the file.

ScheduleOMMetricsWriteTask writes metrics.numKeys to the file in the Timer thread started below.

Making the initial read from file async introduces a race condition: the two threads may now run in the wrong order, in which case OM may write 0 to the file before reading the previous valid value.

I think we can also move the ScheduleOMMetricsWriteTask scheduling code to this runAsync , So the order of the metrics.numKeys init would be the same as before.

ScheduleOMMetricsWriteTask is not a one-time task, it is run periodically.

This sets metrics.numKeys from the file.

ScheduleOMMetricsWriteTask writes metrics.numKeys to the file in the Timer thread started below.

Making the initial read from file async introduces a race condition: the two threads may now run in the wrong order, in which case OM may write 0 to the file before reading the previous valid value.

Thank you for the review. I have moved the metrics.numKeys into the ScheduleOMMetricsWriteTask.

xichen01

@mango-li Thanks for your work on this，a few comments you can refer

xichen01 · 2024-05-15T16:42:56Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

+        if (getMetricsStorageFile().exists()) {
+          OmMetricsInfo metricsInfo = READER.readValue(getMetricsStorageFile());
+          metrics.setNumKeys(metricsInfo.getNumKeys());
+        }


I think we can also move the ScheduleOMMetricsWriteTask scheduling code to this runAsync , So the order of the metrics.numKeys init would be the same as before.

xichen01 · 2024-05-15T16:50:12Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

+      } catch (IOException e) {
+        LOG.warn("Async set om metrics fail.", e);
+      }
+    });


It may be necessary to continue to throw the Exception here.

It may be necessary to continue to throw the Exception here.

Thank you for the review. I have updated the code to throw the Exception here.

guohao-rosicky

Add a construction method for ScheduleOMMetricsWriteTask

  private class ScheduleOMMetricsWriteTask extends TimerTask {
    public ScheduleOMMetricsWriteTask() throws IOException {
      final File metricsStorageFile = getMetricsStorageFile();
      if (metricsStorageFile.exists()) {
        OmMetricsInfo metricsInfo = READER.readValue(metricsStorageFile);
        metrics.setNumKeys(metricsInfo.getNumKeys());
      }
    }

ChenSammi · 2024-05-17T09:35:01Z

@mango-li , thanks on working on this. I have one question, have we measured that how much time can be saved by this async set? Since this metric set looks like a simple operation.

mango-li · 2024-05-20T05:48:02Z

@mango-li , thanks on working on this. I have one question, have we measured that how much time can be saved by this async set? Since this metric set looks like a simple operation.

Thank you for the review. We have 6000 buckets, and they both have link in different volumes, so the async set can save about five seconds of OM startup time.

ChenSammi · 2024-05-20T08:05:45Z

@mango-li , thanks on working on this. I have one question, have we measured that how much time can be saved by this async set? Since this metric set looks like a simple operation.

Thank you for the review. We have 6000 buckets, and they both have link in different volumes, so the async set can save about five seconds of OM startup time.

Does it mean the time saved is actually from "metadataManager.countRowsInTable(metadataManager.getBucketTable()", instead of om metrics set?

mango-li · 2024-05-23T03:40:06Z

@mango-li , thanks on working on this. I have one question, have we measured that how much time can be saved by this async set? Since this metric set looks like a simple operation.

Thank you for the review. We have 6000 buckets, and they both have link in different volumes, so the async set can save about five seconds of OM startup time.

Does it mean the time saved is actually from "metadataManager.countRowsInTable(metadataManager.getBucketTable()", instead of om metrics set?

Yes. The time saved is mainly from metadataManager.countRowsInTable(metadataManager.getVolumeTable()) and metadataManager.countRowsInTable(metadataManager.getBucketTable()), so I changed om metrics set to async, including metrics.numVolumes and metrics.numBuckets.

adoroszlai · 2024-05-23T06:22:38Z

The time saved is mainly from metadataManager.countRowsInTable(metadataManager.getVolumeTable()) and metadataManager.countRowsInTable(metadataManager.getBucketTable())

Volume and bucket tables are fully cached, so we could get row count from cache size by calling:

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java

Lines 453 to 456 in 6f30f2f

    
           public long getEstimatedKeyCount() throws IOException { 
        
             if (cache.getCacheType() == CacheType.FULL_CACHE) { 
        
               return cache.size(); 
        
             }

This could save time by avoiding iteration and key/value conversion.

adoroszlai · 2024-05-23T06:33:24Z

Can asynchronous metrics initialization also interfere with regular operations? E.g. if clients start creating or deleting buckets while async task is still iterating buckets, could numBuckets value be wrong?

guohao-rosicky · 2024-05-23T12:57:11Z

The volume table and bucket table use FullTableCache, CountEstimatedRowsInTable is actually the exact number of rows in the table. Let's keep it synchronized.

Metrics.setNumVolumes(metadataManager.countEstimatedRowsInTable(metadataManager.getvolumeTable()));
Metrics.setNumBuckets(metadataManager.countEstimatedRowsInTable(metadataManager.getBucketTable()));

mango-li · 2024-05-23T13:54:20Z

The time saved is mainly from metadataManager.countRowsInTable(metadataManager.getVolumeTable()) and metadataManager.countRowsInTable(metadataManager.getBucketTable())

Volume and bucket tables are fully cached, so we could get row count from cache size by calling:

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java

Lines 453 to 456 in 6f30f2f

public long getEstimatedKeyCount() throws IOException {

if (cache.getCacheType() == CacheType.FULL_CACHE) {

return cache.size();

}

This could save time by avoiding iteration and key/value conversion.

Good idea! I have changed metrics.numVolumes and metrics.numBuckets using metadataManager.countEstimatedRowsInTable. And still keep om metrics set synchronized.

mango-li · 2024-05-23T13:57:27Z

The volume table and bucket table use FullTableCache, CountEstimatedRowsInTable is actually the exact number of rows in the table. Let's keep it synchronized.
Metrics.setNumVolumes(metadataManager.countEstimatedRowsInTable(metadataManager.getvolumeTable()));
Metrics.setNumBuckets(metadataManager.countEstimatedRowsInTable(metadataManager.getBucketTable()));

Thank you for the review. I have updated the code.

adoroszlai

Thanks @mango-li for updating the patch.

There is similar logic in reloadOMState and restart, might want to update there, too (or even extract common metrics init to avoid duplication).

adoroszlai · 2024-05-27T06:19:26Z

@mango-li can you please let us know your ASF JIRA user name?

HDDS-10862. Async set om metirc when om start

83cedb0

guohao-rosicky requested review from kerneltime, adoroszlai, ChenSammi and xichen01 May 15, 2024 12:08

adoroszlai requested a review from duongkame May 15, 2024 12:13

adoroszlai reviewed May 15, 2024

View reviewed changes

adoroszlai added the metrics label May 15, 2024

xichen01 reviewed May 15, 2024

View reviewed changes

guohao-rosicky reviewed May 16, 2024

View reviewed changes

HDDS-10862. Async set om metirc when om start

11522fe

mango-li requested review from guohao-rosicky, adoroszlai and xichen01 May 23, 2024 03:40

adoroszlai changed the title ~~HDDS-10862. Async set om metirc when om start~~ HDDS-10862. Reduce time spent on initializing metrics during OM start May 23, 2024

HDDS-10862. Reduce time spent on initializing metrics during OM start

7ba0dfc

adoroszlai approved these changes May 23, 2024

View reviewed changes

guohao-rosicky approved these changes May 27, 2024

View reviewed changes

guohao-rosicky merged commit 879c6ca into apache:master May 27, 2024
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-10862. Reduce time spent on initializing metrics during OM start #6682

HDDS-10862. Reduce time spent on initializing metrics during OM start #6682

mango-li commented May 15, 2024

adoroszlai left a comment

adoroszlai May 15, 2024

xichen01 May 15, 2024

adoroszlai May 15, 2024

mango-li May 20, 2024

xichen01 left a comment

xichen01 May 15, 2024

xichen01 May 15, 2024

mango-li May 20, 2024

guohao-rosicky left a comment

ChenSammi commented May 17, 2024 •

edited

mango-li commented May 20, 2024

ChenSammi commented May 20, 2024

mango-li commented May 23, 2024

adoroszlai commented May 23, 2024

adoroszlai commented May 23, 2024

guohao-rosicky commented May 23, 2024

mango-li commented May 23, 2024

mango-li commented May 23, 2024

adoroszlai left a comment

adoroszlai commented May 27, 2024

HDDS-10862. Reduce time spent on initializing metrics during OM start #6682

HDDS-10862. Reduce time spent on initializing metrics during OM start #6682

Conversation

mango-li commented May 15, 2024

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

adoroszlai left a comment

Choose a reason for hiding this comment

adoroszlai May 15, 2024

Choose a reason for hiding this comment

xichen01 May 15, 2024

Choose a reason for hiding this comment

adoroszlai May 15, 2024

Choose a reason for hiding this comment

mango-li May 20, 2024

Choose a reason for hiding this comment

xichen01 left a comment

Choose a reason for hiding this comment

xichen01 May 15, 2024

Choose a reason for hiding this comment

xichen01 May 15, 2024

Choose a reason for hiding this comment

mango-li May 20, 2024

Choose a reason for hiding this comment

guohao-rosicky left a comment

Choose a reason for hiding this comment

ChenSammi commented May 17, 2024 • edited

mango-li commented May 20, 2024

ChenSammi commented May 20, 2024

mango-li commented May 23, 2024

adoroszlai commented May 23, 2024

adoroszlai commented May 23, 2024

guohao-rosicky commented May 23, 2024

mango-li commented May 23, 2024

mango-li commented May 23, 2024

adoroszlai left a comment

Choose a reason for hiding this comment

adoroszlai commented May 27, 2024

ChenSammi commented May 17, 2024 •

edited