You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "devabhishekpal (via GitHub)" <gi...@apache.org> on 2025/02/07 11:41:45 UTC
[PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
devabhishekpal opened a new pull request, #7835:
URL: https://github.com/apache/ozone/pull/7835
## What changes were proposed in this pull request?
HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon
Please describe your PR in detail:
This PR is to extend the metrics present in Recon currently.
As a part of this PR we are adding metrics in `OzoneManagerServiceProviderImpl` and ReconOmTasks.
For OzoneManagerServiceProviderImpl we add the following metrics:
- The time taken for Delta request
- The time taken for writing the delta updates
- Fix the metric for delta update failed requests
For ReconOmTasks we are adding the following metrics:
- The number of times process() and reprocess() is called
- The number of times process() and reprocess() encounters failures
- The amount of time taken for a run of process() and reprocess()
- The number of times an OM task is writing to the DB (this can be RocksDB for some tasks and DerbyDb for some)
- The latency/time taken to write to the DB
- The number of write, failures and latency while trying to write the NSSummary map to RocksDB
In NSSummaryTask we are further adding metrics at the level of FSO, OBS, Legacy type processing where we have
- NSSummaryTask level count, time taken for process() and reprocess() counts - this will encapsulate the whole sub-threads/tasks that are running for FSO, OBS and Legacy
- Then we are further drilling down to the subtask level i.e OBS, FSO and Legacy and adding the count of calls and failures for processing and reprocess and the time taken for the same
- We are also adding the metrics on how many time a
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12242
## How was this patch tested?
Patch was tested manually via prometheus
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #7835: HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon
URL: https://github.com/apache/ozone/pull/7835
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #7835:
URL: https://github.com/apache/ozone/pull/7835#discussion_r1959104827
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/ReconOmTaskMetrics.java:
##########
@@ -0,0 +1,104 @@
+package org.apache.hadoop.ozone.recon.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsCollector;
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.metrics2.MetricsSource;
+import org.apache.hadoop.metrics2.MetricsTag;
+import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.lib.Interns;
+import org.apache.hadoop.metrics2.lib.MetricsRegistry;
+import org.apache.hadoop.metrics2.lib.MutableCounterLong;
+import org.apache.hadoop.metrics2.lib.MutableRate;
+
+/**
+ * This interface is to be implemented by the various metrics for different tasks
+ */
+public abstract class ReconOmTaskMetrics implements MetricsSource {
+
+ private final MetricsRegistry registry = new MetricsRegistry("ReconOMTasks");
+ private final String taskName;
+ private final String source;
+
+ private @Metric MutableCounterLong reprocessCount;
+ private @Metric MutableCounterLong reprocessFailedCount;
+ private @Metric MutableRate reprocessLatency;
+
+ private @Metric MutableCounterLong processCount;
+ private @Metric MutableCounterLong processFailedCount;
+ private @Metric MutableRate processLatency;
+
+ protected ReconOmTaskMetrics(String taskName, String source) {
+ this.taskName = taskName;
+ this.source = source;
+ }
+
+ /**
+ * Update the number of reprocess() method calls by the task
+ */
+ public void incrTaskReprocessCount() {
Review Comment:
So in this case we are incrementing the count in the beginning to track how many times the method was called.
There is another counter `reprocessFailedCount` which tracks the number of times the method failed/encountered exception.
The number of successful runs can be found by `reprocessCount - reprocessFailedCount`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2775390243
/pending conflicts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2664093552
@devmadhuu most files are affected by import reordering (HDDS-12282), but `OmTableInsightTask` is also conflicting due to HDDS-12110. Can you please help @devabhishekpal resolve the conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2648698545
Review is in progress
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2645837140
Thanks @devabhishekpal for the patch. Please take a look at failed checks:
https://github.com/devabhishekpal/ozone/actions/runs/13209740038/job/36880830920
https://github.com/devabhishekpal/ozone/actions/runs/13209740038/job/36880831324
https://github.com/devabhishekpal/ozone/actions/runs/13209740038/job/36880831495
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #7835:
URL: https://github.com/apache/ozone/pull/7835#discussion_r1950249835
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -137,14 +150,11 @@ public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
return new ImmutablePair<>(getTaskName(), false);
}
- LOG.debug("Completed 'reprocess' of ContainerKeyMapperTask.");
- Instant end = Instant.now();
- long duration = Duration.between(start, end).toMillis();
- LOG.debug("It took me {} seconds to process {} keys.",
- (double) duration / 1000.0, omKeyCount);
+ LOG.debug("Completed 'reprocess' of ContainerKeyMapperTask on {} keys.", omKeyCount);
} catch (IOException ioEx) {
LOG.error("Unable to populate Container Key data in Recon DB. ",
ioEx);
+ metrics.incrTaskReprocessFailureCount();
Review Comment:
Pls check , we have multiple failure points for `reprocess` in the task
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -87,45 +96,49 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
long omKeyCount = 0;
-
+ metrics.incrTaskReprocessCount();
// In-memory maps for fast look up and batch write
// (container, key) -> count
Map<ContainerKeyPrefix, Integer> containerKeyMap = new HashMap<>();
// containerId -> key count
Map<Long, Long> containerKeyCountMap = new HashMap<>();
try {
LOG.debug("Starting a 'reprocess' run of ContainerKeyMapperTask.");
- Instant start = Instant.now();
// initialize new container DB
reconContainerMetadataManager
.reinitWithNewContainerDataFromOm(new HashMap<>());
// loop over both key table and file table
- for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
- BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
- // (HDDS-8580) Since "reprocess" iterate over the whole key table,
- // containerKeyMap needs to be incrementally flushed to DB based on
- // configured batch threshold.
- // containerKeyCountMap can be flushed at the end since the number
- // of containers in a cluster will not have significant memory overhead.
- Table<String, OmKeyInfo> omKeyInfoTable =
- omMetadataManager.getKeyTable(layout);
- try (
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
- keyIter = omKeyInfoTable.iterator()) {
- while (keyIter.hasNext()) {
- Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
- OmKeyInfo omKeyInfo = kv.getValue();
- handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
- containerKeyCountMap);
- if (!checkAndCallFlushToDB(containerKeyMap)) {
- LOG.error("Unable to flush containerKey information to the DB");
- return new ImmutablePair<>(getTaskName(), false);
+ long startTime = Time.monotonicNow();
Review Comment:
Could this start time be moved before `reinitWithNewContainerDataFromOm` ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -249,31 +264,37 @@ public Pair<String, Boolean> process(OMUpdateEventBatch events) {
} catch (IOException e) {
LOG.error("Unexpected exception while updating key data : {} ",
updatedKey, e);
+ metrics.incrTaskProcessFailureCount();
Review Comment:
What about runtime exceptions which are thrown back to caller of `process` ? Runtime error are most common in ReconOM task framework.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -193,6 +204,7 @@ public Collection<String> getTaskTables() {
@Override
public Pair<String, Boolean> process(OMUpdateEventBatch events) {
+ metrics.incrTaskProcessCount();
Review Comment:
same comment as given for `reprocess`
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -519,6 +523,7 @@ boolean innerGetAndApplyDeltaUpdatesFromOM(long fromSequenceNumber,
}
}
}
+ metrics.updateDeltaUpdateWriteLatency(Time.monotonicNow() - startTime);
Review Comment:
We should capture separate metrics for converting the OM DB updates into recon events and separate for writing/applying OM DB updates to Recon OM snapshot DB.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -220,16 +232,19 @@ public Pair<String, Boolean> process(OMUpdateEventBatch events) {
try {
switch (omdbUpdateEvent.getAction()) {
case PUT:
+ metrics.incrPutKeyEventCount();
handlePutOMKeyEvent(updatedKey, updatedKeyValue, containerKeyMap,
containerKeyCountMap, deletedKeyCountList);
break;
case DELETE:
+ metrics.incrDeleteKeyEventCount();
handleDeleteOMKeyEvent(updatedKey, containerKeyMap,
containerKeyCountMap, deletedKeyCountList);
break;
case UPDATE:
+ metrics.incrUpdateKeyEventCount();
Review Comment:
Metrics capture for all `PUT`, `DELETE` and `UPDATE` events should be moved after the actual processing of event.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -296,12 +318,15 @@ private void writeToTheDB(Map<ContainerKeyPrefix, Integer> containerKeyMap,
reconContainerMetadataManager
.batchDeleteContainerMapping(rdbBatchOperation, key);
} catch (IOException e) {
+ metrics.incrTaskWriteToDBFailureCount();
Review Comment:
We are incrementing the same write to db failure count metric for 3 different types of data write to DB, is this expected ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/ReconOmTaskMetrics.java:
##########
@@ -0,0 +1,104 @@
+package org.apache.hadoop.ozone.recon.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsCollector;
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.metrics2.MetricsSource;
+import org.apache.hadoop.metrics2.MetricsTag;
+import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.lib.Interns;
+import org.apache.hadoop.metrics2.lib.MetricsRegistry;
+import org.apache.hadoop.metrics2.lib.MutableCounterLong;
+import org.apache.hadoop.metrics2.lib.MutableRate;
+
+/**
+ * This interface is to be implemented by the various metrics for different tasks
+ */
+public abstract class ReconOmTaskMetrics implements MetricsSource {
+
+ private final MetricsRegistry registry = new MetricsRegistry("ReconOMTasks");
+ private final String taskName;
+ private final String source;
+
+ private @Metric MutableCounterLong reprocessCount;
+ private @Metric MutableCounterLong reprocessFailedCount;
+ private @Metric MutableRate reprocessLatency;
+
+ private @Metric MutableCounterLong processCount;
+ private @Metric MutableCounterLong processFailedCount;
+ private @Metric MutableRate processLatency;
+
+ protected ReconOmTaskMetrics(String taskName, String source) {
+ this.taskName = taskName;
+ this.source = source;
+ }
+
+ /**
+ * Update the number of reprocess() method calls by the task
+ */
+ public void incrTaskReprocessCount() {
Review Comment:
Shouldn't this metric be captured at the end of `reprocess` ?because `reprocess` may fail, but we still incremented this metric ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -249,31 +264,37 @@ public Pair<String, Boolean> process(OMUpdateEventBatch events) {
} catch (IOException e) {
LOG.error("Unexpected exception while updating key data : {} ",
updatedKey, e);
+ metrics.incrTaskProcessFailureCount();
return new ImmutablePair<>(getTaskName(), false);
}
}
+ metrics.updateTaskProcessLatency(Time.monotonicNow() - startTime);
+
try {
writeToTheDB(containerKeyMap, containerKeyCountMap, deletedKeyCountList);
} catch (IOException e) {
LOG.error("Unable to write Container Key Prefix data in Recon DB.", e);
+ metrics.incrTaskWriteToDBFailureCount();
return new ImmutablePair<>(getTaskName(), false);
}
- LOG.debug("{} successfully processed {} OM DB update event(s) in {} milliseconds.",
- getTaskName(), eventCount, (System.currentTimeMillis() - startTime));
+ LOG.debug("{} successfully processed {} OM DB update event(s)", getTaskName(), eventCount);
return new ImmutablePair<>(getTaskName(), true);
}
private void writeToTheDB(Map<ContainerKeyPrefix, Integer> containerKeyMap,
Map<Long, Long> containerKeyCountMap,
List<ContainerKeyPrefix> deletedContainerKeyList)
throws IOException {
+ metrics.incrTaskWriteToDBCount();
Review Comment:
IMO, should be moved at the end in finally.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -87,45 +96,49 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
long omKeyCount = 0;
-
+ metrics.incrTaskReprocessCount();
// In-memory maps for fast look up and batch write
// (container, key) -> count
Map<ContainerKeyPrefix, Integer> containerKeyMap = new HashMap<>();
// containerId -> key count
Map<Long, Long> containerKeyCountMap = new HashMap<>();
try {
LOG.debug("Starting a 'reprocess' run of ContainerKeyMapperTask.");
- Instant start = Instant.now();
// initialize new container DB
reconContainerMetadataManager
.reinitWithNewContainerDataFromOm(new HashMap<>());
// loop over both key table and file table
- for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
- BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
- // (HDDS-8580) Since "reprocess" iterate over the whole key table,
- // containerKeyMap needs to be incrementally flushed to DB based on
- // configured batch threshold.
- // containerKeyCountMap can be flushed at the end since the number
- // of containers in a cluster will not have significant memory overhead.
- Table<String, OmKeyInfo> omKeyInfoTable =
- omMetadataManager.getKeyTable(layout);
- try (
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
- keyIter = omKeyInfoTable.iterator()) {
- while (keyIter.hasNext()) {
- Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
- OmKeyInfo omKeyInfo = kv.getValue();
- handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
- containerKeyCountMap);
- if (!checkAndCallFlushToDB(containerKeyMap)) {
- LOG.error("Unable to flush containerKey information to the DB");
- return new ImmutablePair<>(getTaskName(), false);
+ long startTime = Time.monotonicNow();
+ try {
+ for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
+ BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
+ // (HDDS-8580) Since "reprocess" iterate over the whole key table,
+ // containerKeyMap needs to be incrementally flushed to DB based on
+ // configured batch threshold.
+ // containerKeyCountMap can be flushed at the end since the number
+ // of containers in a cluster will not have significant memory overhead.
+ Table<String, OmKeyInfo> omKeyInfoTable =
+ omMetadataManager.getKeyTable(layout);
+ try (
+ TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+ keyIter = omKeyInfoTable.iterator()) {
+ while (keyIter.hasNext()) {
+ Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
+ OmKeyInfo omKeyInfo = kv.getValue();
+ handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
+ containerKeyCountMap);
+ if (!checkAndCallFlushToDB(containerKeyMap)) {
+ LOG.error("Unable to flush containerKey information to the DB");
+ return new ImmutablePair<>(getTaskName(), false);
+ }
+ omKeyCount++;
}
- omKeyCount++;
}
}
+ } finally {
Review Comment:
For capturing task latency and excluding DB write latency, this is fine, but DB flush latency should be captured on final flush and not every intermediate flush.As of now it looks like db flush latency also being added in task reprocess latency which is wrong. Do we want to capture with DB write latency ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -87,45 +96,49 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
long omKeyCount = 0;
-
+ metrics.incrTaskReprocessCount();
Review Comment:
Shouldn't this metric be captured at the end of `reprocess` ?because `reprocess` may fail, but we still incremented this metric ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2650427726
@devabhishekpal pls include a doc which documents metrics being added at each level and share the doc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7835:
URL: https://github.com/apache/ozone/pull/7835#issuecomment-2829116903
Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author.
It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.
It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.
If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12223. Add metrics for OzoneManager SPI and OMTask in Recon [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #7835:
URL: https://github.com/apache/ozone/pull/7835#discussion_r1959134820
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -87,45 +96,49 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
long omKeyCount = 0;
-
+ metrics.incrTaskReprocessCount();
// In-memory maps for fast look up and batch write
// (container, key) -> count
Map<ContainerKeyPrefix, Integer> containerKeyMap = new HashMap<>();
// containerId -> key count
Map<Long, Long> containerKeyCountMap = new HashMap<>();
try {
LOG.debug("Starting a 'reprocess' run of ContainerKeyMapperTask.");
- Instant start = Instant.now();
// initialize new container DB
reconContainerMetadataManager
.reinitWithNewContainerDataFromOm(new HashMap<>());
// loop over both key table and file table
- for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
- BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
- // (HDDS-8580) Since "reprocess" iterate over the whole key table,
- // containerKeyMap needs to be incrementally flushed to DB based on
- // configured batch threshold.
- // containerKeyCountMap can be flushed at the end since the number
- // of containers in a cluster will not have significant memory overhead.
- Table<String, OmKeyInfo> omKeyInfoTable =
- omMetadataManager.getKeyTable(layout);
- try (
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
- keyIter = omKeyInfoTable.iterator()) {
- while (keyIter.hasNext()) {
- Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
- OmKeyInfo omKeyInfo = kv.getValue();
- handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
- containerKeyCountMap);
- if (!checkAndCallFlushToDB(containerKeyMap)) {
- LOG.error("Unable to flush containerKey information to the DB");
- return new ImmutablePair<>(getTaskName(), false);
+ long startTime = Time.monotonicNow();
+ try {
+ for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
+ BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
+ // (HDDS-8580) Since "reprocess" iterate over the whole key table,
+ // containerKeyMap needs to be incrementally flushed to DB based on
+ // configured batch threshold.
+ // containerKeyCountMap can be flushed at the end since the number
+ // of containers in a cluster will not have significant memory overhead.
+ Table<String, OmKeyInfo> omKeyInfoTable =
+ omMetadataManager.getKeyTable(layout);
+ try (
+ TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+ keyIter = omKeyInfoTable.iterator()) {
+ while (keyIter.hasNext()) {
+ Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
+ OmKeyInfo omKeyInfo = kv.getValue();
+ handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
+ containerKeyCountMap);
+ if (!checkAndCallFlushToDB(containerKeyMap)) {
+ LOG.error("Unable to flush containerKey information to the DB");
+ return new ImmutablePair<>(getTaskName(), false);
+ }
+ omKeyCount++;
}
- omKeyCount++;
}
}
+ } finally {
Review Comment:
So for this:
- Writing to DB is a part of the reprocess task as well, so I think we should add the DB write latency as a part of the overall time taken for reprocess. We can figure out the reprocess time taken from the total time taken to run reprocess() at time `x` and finding difference from the DB write latency at the same time `x`. But in my opinion if we separate out the DB write latency then:
- We will have to do something like:
```
long reprocessLatency = 0L;
for (BucketLayout layout : Arrays.asList(BucketLayout.LEGACY,
BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
...
try (
TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
keyIter = omKeyInfoTable.iterator()) {
while (keyIter.hasNext()) {
...
long startTime = Time.monotonicNow();
handleKeyReprocess(kv.getKey(), omKeyInfo, containerKeyMap,
containerKeyCountMap);
reprocessLatency += Time.monotonicNow() - startTime;
// DB flush part
if (!checkAndCallFlushToDB(containerKeyMap)) {
...
}
...
}
}
}
...
metrics.updateTaskReprocessLatency(reprocessLatency);
```
But this add extra code for calculating the `handleKeyReprocess()` duration.
- For the point of capturing the final flush it is correct, but then we might miss out scenarios like some flush taking longer time. Say we flushed the data three times, out of which for some reason the 2nd flush took a lot longer.
If we only track the final flush we will miss this information. Same for if we aggregate the total time taken for all the flushes. In aggregation we can do something like the above where we keep adding the flush latency to a variable and then write the final sum but we will miss if some intermediate flush took a lot longer than others.
Inputs on this would be great
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org