Tejaskriya commented on code in PR #8896:
URL: https://github.com/apache/ozone/pull/8896#discussion_r2256120986
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMPerformanceMetrics.java:
##########
@@ -291,6 +306,26 @@ public void setDeleteKeysAclCheckLatencyNs(long
latencyInNs) {
public MutableRate getDeleteKeyResolveBucketAndAclCheckLatencyNs() {
return deleteKeyResolveBucketAndAclCheckLatencyNs;
}
+
+ public MutableRate getCreateKeyResolveBucketAndAclCheckLatencyNs() {
+ return createKeyResolveBucketAndAclCheckLatencyNs;
+ }
+
+ public void addCreateKeyQuotaCheckLatencyNs(long latencyInNs) {
+ createKeyQuotaCheckLatencyNs.add(latencyInNs);
+ }
+
+ public MutableRate getCreateKeyAllocateBlockLatencyNs() {
+ return createKeyAllocateBlockLatencyNs;
+ }
+
+ public void setCreateKeyFailureLatencyNs(long latencyInNs) {
+ createKeyFailureLatencyNs.add(latencyInNs);
Review Comment:
The method name and the actual operation are mismatched, set and add is
confusing. Although other methods in this class have used just a pattern, I
would suggest to keep the method name as "add..."
```suggestion
public void addCreateKeyFailureLatencyNs(long latencyInNs) {
createKeyFailureLatencyNs.add(latencyInNs);
```
Perhaps is there a different reason why the method is
`addCreateKeyQuotaCheckLatencyNs` for quota and `setCreateKeyFailureLatencyNs`
for createKey?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java:
##########
@@ -174,9 +178,11 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
long preAllocatedSpace =
newLocationList.size() * ozoneManager.getScmBlockSize() * repConfig
.getRequiredNodes();
+ long startTime = Time.monotonicNowNanos();
Review Comment:
Same as above:
```suggestion
long quotaCheckStartTime = Time.monotonicNowNanos();
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java:
##########
@@ -91,6 +93,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
Result result;
List<OmDirectoryInfo> missingParentInfos;
int numKeysCreated = 0;
+ final OMPerformanceMetrics perfMetrics = ozoneManager.getPerfMetrics();
+ long startNanos = Time.monotonicNowNanos();
Review Comment:
Multiple variables named with the prefix "start.." will make the code not
very readable. Can we have more self-explanatory names like:
```suggestion
long createKeyStartTime = Time.monotonicNowNanos();
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java:
##########
@@ -212,6 +219,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
Result result = null;
List<OmKeyInfo> missingParentInfos = null;
int numMissingParents = 0;
+ final OMPerformanceMetrics perfMetrics = ozoneManager.getPerfMetrics();
+ long startNanos = Time.monotonicNowNanos();
Review Comment:
The naming of variables can be a bit more descriptive:
```suggestion
long createKeyStartTime = Time.monotonicNowNanos();
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMPerformanceMetrics.java:
##########
@@ -291,6 +306,26 @@ public void setDeleteKeysAclCheckLatencyNs(long
latencyInNs) {
public MutableRate getDeleteKeyResolveBucketAndAclCheckLatencyNs() {
return deleteKeyResolveBucketAndAclCheckLatencyNs;
}
+
+ public MutableRate getCreateKeyResolveBucketAndAclCheckLatencyNs() {
+ return createKeyResolveBucketAndAclCheckLatencyNs;
+ }
+
+ public void addCreateKeyQuotaCheckLatencyNs(long latencyInNs) {
+ createKeyQuotaCheckLatencyNs.add(latencyInNs);
+ }
+
+ public MutableRate getCreateKeyAllocateBlockLatencyNs() {
+ return createKeyAllocateBlockLatencyNs;
+ }
+
+ public void setCreateKeyFailureLatencyNs(long latencyInNs) {
+ createKeyFailureLatencyNs.add(latencyInNs);
+ }
+
+ public void setCreateKeySuccessLatencyNs(long latencyInNs) {
+ createKeySuccessLatencyNs.add(latencyInNs);
Review Comment:
Same as above.
```suggestion
public void addCreateKeySuccessLatencyNs(long latencyInNs) {
createKeySuccessLatencyNs.add(latencyInNs);
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java:
##########
@@ -302,9 +311,11 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
* ozoneManager.getScmBlockSize()
* replicationConfig.getRequiredNodes();
// check bucket and volume quota
+ long startTime = Time.monotonicNowNanos();
Review Comment:
Same as above
```suggestion
long quotaCheckStartTime = Time.monotonicNowNanos();
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
