You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "muskan1012 (via GitHub)" <gi...@apache.org> on 2023/12/07 12:32:01 UTC
[PR] HDDS-9874. Introduce Metrics for listKeys Dashboard [ozone]
muskan1012 opened a new pull request, #5745:
URL: https://github.com/apache/ozone/pull/5745
## What changes were proposed in this pull request?
Introduced following metrics for listKey operations:
- listKeysAveragePagination
- listKeysOpsPerSec
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9874
## How was this patch tested?
manually tested it on a cluster
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1429634872
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java:
##########
@@ -56,7 +55,7 @@ public class TestOMSnapshotDeleteResponse {
@TempDir
private Path folder;
- private OMMetadataManager omMetadataManager;
+ private OmMetadataManagerImpl omMetadataManager;
Review Comment:
We can retain `omMetadataManager` object to be of type `OMMetadataManager` as is.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotDeleteResponse.java:
##########
@@ -40,7 +39,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
-
+import org.apache.hadoop.ozone.om.OMConfigKeys;
Review Comment:
I think we can avoid repositioning of imports such as this one.
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1420568711
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMPerformanceMetrics.java:
##########
@@ -113,6 +113,11 @@ public static void unregister() {
@Metric(about = "Ratis local command execution latency in nano seconds")
private MutableRate validateAndUpdateCacneLatencyNs;
+ @Metric(about = "average pagination for listKeys")
+ private MutableRate listKeysAveragePagination;
+
+ @Metric(about = "ops per second for listKeys")
+ private MutableRate listKeysOpsPerSec;
Review Comment:
We have the Latency metric `listKeysLatencyNs`, maybe we just need to add a cumulative number of listKeys metric(such as: listKeysSize), which we can calculate to get `listKeysOpsPerSec` and `listKeysAveragePagination`.
https://github.com/apache/ozone/blob/0e91b2aa81e32320f7922261fc209c1d8d91cf9b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMPerformanceMetrics.java#L92-L93
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-1845266492
@muskan1012 There are checkstyle and findbugs failures. It would be nice to wait with PR creation until a clean CI run in your fork is completed.
https://github.com/muskan1012/ozone/actions/runs/7128100497
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1657123142
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -328,28 +329,46 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
*/
public OmMetadataManagerImpl(OzoneConfiguration conf,
OzoneManager ozoneManager) throws IOException {
- this.ozoneManager = ozoneManager;
+ init(conf, ozoneManager);
+ this.lock = new OzoneManagerLock(conf);
+ this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = null;
+ start(conf);
+ }
+
+ public OmMetadataManagerImpl(OzoneConfiguration conf,
+ OzoneManager ozoneManager,
+ OMPerformanceMetrics perfMetrics)
+ throws IOException {
+ init(conf, ozoneManager);
this.lock = new OzoneManagerLock(conf);
- // TODO: This is a temporary check. Once fully implemented, all OM state
- // change should go through Ratis - be it standalone (for non-HA) or
- // replicated (for HA).
- isRatisEnabled = conf.getBoolean(
- OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
- OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = perfMetrics;
// For test purpose only
ignorePipelineinKey = conf.getBoolean(
- "ozone.om.ignore.pipeline", Boolean.TRUE);
+ "ozone.om.ignore.pipeline", Boolean.TRUE);
start(conf);
}
-
+ private void init(OzoneConfiguration conf, OzoneManager manager) {
+ this.ozoneManager = manager;
+ // TODO: This is a temporary check. Once fully implemented, all OM state
+ // change should go through Ratis - be it standalone (for non-HA) or
+ // replicated (for HA).
Review Comment:
Minor nit: Can we refactor the alignment of the Java comments under `init()` from a readability standpoint?
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1568205148
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1258,10 +1279,22 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
break;
}
}
+ stopNano = Time.monotonicNowNanos();
}
-
boolean isTruncated = cacheKeyMap.size() > maxKeys;
-
+ if (perfMetrics != null) {
+ long averagePagination;
Review Comment:
```suggestion
long keyCount;
```
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1668099227
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1258,10 +1279,22 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
break;
}
}
+ stopNano = Time.monotonicNowNanos();
}
-
boolean isTruncated = cacheKeyMap.size() > maxKeys;
-
+ if (perfMetrics != null) {
+ long averagePagination;
Review Comment:
As per discussion, we decided to go with the name `averagePagination` for now. We may consider changing the same in a separate PR if needed, thanks!
@muskan1012
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1657147481
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -328,28 +329,46 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
*/
public OmMetadataManagerImpl(OzoneConfiguration conf,
OzoneManager ozoneManager) throws IOException {
- this.ozoneManager = ozoneManager;
+ init(conf, ozoneManager);
+ this.lock = new OzoneManagerLock(conf);
+ this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = null;
+ start(conf);
+ }
+
+ public OmMetadataManagerImpl(OzoneConfiguration conf,
+ OzoneManager ozoneManager,
+ OMPerformanceMetrics perfMetrics)
+ throws IOException {
+ init(conf, ozoneManager);
this.lock = new OzoneManagerLock(conf);
- // TODO: This is a temporary check. Once fully implemented, all OM state
- // change should go through Ratis - be it standalone (for non-HA) or
- // replicated (for HA).
- isRatisEnabled = conf.getBoolean(
- OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
- OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = perfMetrics;
// For test purpose only
ignorePipelineinKey = conf.getBoolean(
- "ozone.om.ignore.pipeline", Boolean.TRUE);
+ "ozone.om.ignore.pipeline", Boolean.TRUE);
start(conf);
}
-
+ private void init(OzoneConfiguration conf, OzoneManager manager) {
+ this.ozoneManager = manager;
+ // TODO: This is a temporary check. Once fully implemented, all OM state
+ // change should go through Ratis - be it standalone (for non-HA) or
+ // replicated (for HA).
Review Comment:
`init()` will be hopefully removed and original code restored.
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-1850524857
cc @tanvipenumudy can you please take a look as well?
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-2015940018
@muskan1012 can you please rebase this PR?
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1551284786
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/bucket/TestOMBucketDeleteResponse.java:
##########
@@ -100,7 +102,7 @@ public void testAddToDBBatch() throws Exception {
omMetadataManager.getStore().commitBatchOperation(batchOperation);
assertNull(omMetadataManager.getBucketTable().get(
- omMetadataManager.getBucketKey(volumeName, bucketName)));
+ omMetadataManager.getBucketKey(volumeName, bucketName)));
Review Comment:
Same as the previous comments.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithDummyResponse.java:
##########
@@ -163,14 +164,14 @@ private static class OMDummyCreateBucketResponse extends OMClientResponse {
private final OmBucketInfo omBucketInfo;
OMDummyCreateBucketResponse(OmBucketInfo omBucketInfo,
- OMResponse omResponse) {
+ OMResponse omResponse) {
Review Comment:
Minor nit: Let's stick to the original indent.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerDoubleBufferWithDummyResponse.java:
##########
@@ -163,14 +164,14 @@ private static class OMDummyCreateBucketResponse extends OMClientResponse {
private final OmBucketInfo omBucketInfo;
OMDummyCreateBucketResponse(OmBucketInfo omBucketInfo,
- OMResponse omResponse) {
+ OMResponse omResponse) {
super(omResponse);
this.omBucketInfo = omBucketInfo;
}
@Override
public void addToDBBatch(OMMetadataManager omMetadataManager,
- BatchOperation batchOperation) throws IOException {
+ BatchOperation batchOperation) throws IOException {
Review Comment:
Minor nit: Here as well, we can stick with the original indent.
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-1880462440
Thank you @muskan1012 for updating the patch, could you please take a look at the [build failure](https://github.com/muskan1012/ozone/actions/runs/7406652071/job/20151435227)?
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-1874380809
@muskan1012 can you please rebase the change and address 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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-2175251937
@muskan1012, the failed checks appear related to the changes (`NPE` while initializing a new `OmMetadataManager`). Could you please address this issue similarly to how the previous test failures were handled?
Could you also please rebase the patch? Thanks!
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy merged PR #5745:
URL: https://github.com/apache/ozone/pull/5745
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "muskan1012 (via GitHub)" <gi...@apache.org>.
muskan1012 commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1668461342
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMPerformanceMetrics.java:
##########
@@ -113,6 +113,11 @@ public static void unregister() {
@Metric(about = "Ratis local command execution latency in nano seconds")
private MutableRate validateAndUpdateCacneLatencyNs;
+ @Metric(about = "average pagination for listKeys")
+ private MutableRate listKeysAveragePagination;
+
+ @Metric(about = "ops per second for listKeys")
+ private MutableRate listKeysOpsPerSec;
Review Comment:
I implemented the cumulative number of listKeys using keyCount, which is set to maxKeys if truncated or cacheKeyMap.size() otherwise.
listKeysAveragePagination is set using keyCount.
listKeysOpsPerSec is calculated as keyCount / ((Time.monotonicNowNanos() - startNanos) / 1_000_000_000.0f).
Added latency tracking with addListKeysReadFromRocksDbLatencyNs.
This approach captures the metrics as you suggested.
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1668099227
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1258,10 +1279,22 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
break;
}
}
+ stopNano = Time.monotonicNowNanos();
}
-
boolean isTruncated = cacheKeyMap.size() > maxKeys;
-
+ if (perfMetrics != null) {
+ long averagePagination;
Review Comment:
As per discussion, we decided to go with the name `averagePagination` for now. We may consider changing the same in a separate PR if needed, thanks!
@muskan1012
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #5745:
URL: https://github.com/apache/ozone/pull/5745#issuecomment-2185573360
> **How was this patch tested?**
> _manually tested it on a cluster_
@muskan1012, it would help if you could also please share the updated list keys dashboard (or prometheus/graphana metrics - whichever works!) once all changes are incorporated, thanks.
--
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-9874. Introduce Metrics for listKeys Dashboard [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5745:
URL: https://github.com/apache/ozone/pull/5745#discussion_r1447476466
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -336,19 +337,38 @@ public OmMetadataManagerImpl(OzoneConfiguration conf,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = ozoneManager.getPerfMetrics();
// For test purpose only
ignorePipelineinKey = conf.getBoolean(
"ozone.om.ignore.pipeline", Boolean.TRUE);
start(conf);
}
+ public OmMetadataManagerImpl(OzoneConfiguration conf,
+ OzoneManager ozoneManager,
+ OMPerformanceMetrics perfMetrics)
+ throws IOException {
+ this.ozoneManager = ozoneManager;
+ this.lock = new OzoneManagerLock(conf);
+ isRatisEnabled = conf.getBoolean(
+ OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
+ OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
+ this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = perfMetrics;
+ // For test purpose only
+ ignorePipelineinKey = conf.getBoolean(
+ "ozone.om.ignore.pipeline", Boolean.TRUE);
+ start(conf);
+ }
Review Comment:
Please don't duplicate code from the other constructor.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -336,19 +337,38 @@ public OmMetadataManagerImpl(OzoneConfiguration conf,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
+ this.perfMetrics = ozoneManager.getPerfMetrics();
Review Comment:
I think this causes NPE in some tests, e.g.:
```
Error: org.apache.hadoop.ozone.fsck.TestContainerMapper.testContainerMapper -- Time elapsed: 0.029 s <<< ERROR!
java.lang.NullPointerException
at org.apache.hadoop.ozone.om.OmMetadataManagerImpl.<init>(OmMetadataManagerImpl.java:340)
at org.apache.hadoop.ozone.fsck.ContainerMapper.parseOmDB(ContainerMapper.java:80)
```
```
Error: org.apache.hadoop.fs.ozone.TestOzoneFileSystemPrefixParser.testPrefixParsePath -- Time elapsed: 4.110 s <<< ERROR!
java.lang.NullPointerException
at org.apache.hadoop.ozone.om.OmMetadataManagerImpl.<init>(OmMetadataManagerImpl.java:340)
at org.apache.hadoop.ozone.debug.PrefixParser.parse(PrefixParser.java:132)
at org.apache.hadoop.fs.ozone.TestOzoneFileSystemPrefixParser.verifyPrefixParsePath(TestOzoneFileSystemPrefixParser.java:164)
at org.apache.hadoop.fs.ozone.TestOzoneFileSystemPrefixParser.testPrefixParsePath(TestOzoneFileSystemPrefixParser.java:106)
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/bucket/TestOMBucketDeleteResponse.java:
##########
@@ -99,8 +101,8 @@ public void testAddToDBBatch() throws Exception {
// Do manual commit and see whether addToBatch is successful or not.
omMetadataManager.getStore().commitBatchOperation(batchOperation);
- assertNull(omMetadataManager.getBucketTable().get(
- omMetadataManager.getBucketKey(volumeName, bucketName)));
+ assertEquals(null, omMetadataManager.getBucketTable().get(
+ omMetadataManager.getBucketKey(volumeName, bucketName)));
Review Comment:
Let's keep `assertNull`.
--
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