adoroszlai commented on code in PR #9626:
URL: https://github.com/apache/ozone/pull/9626#discussion_r2687251728


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java:
##########
@@ -189,4 +190,12 @@ void incNumPipelineContainSameDatanodes() {
   public void updatePipelineCreationLatencyNs(long startNanos) {
     pipelineCreationLatencyNs.add(Time.monotonicNowNanos() - startNanos);
   }
+
+  /**
+   * Return number of blocks allocated across all pipelines.
+   */
+  @VisibleForTesting

Review Comment:
   I'd rather omit this annotation.  It does not add much value, and is not 
enforced (gets outdated when new code starts using it in prod).  If you 
strongly feel the method should not be used in prod, it's better to include 
`ForTesting` in its name.  It makes the intention more obvious for others at 
the call site, not only at the definition.  For a metric, I don't think that's 
necessary.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java:
##########
@@ -144,9 +145,17 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
       //  As for a client for the first time this can be executed on any OM,
       //  till leader is identified.
       UserInfo userInfo = getUserInfo();

Review Comment:
   Not introduced in this patch, but `getUserInfo()` is also called at the end 
of this method.  We should store and reuse `userInfo` in both places.



-- 
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]

Reply via email to