[ 
https://issues.apache.org/jira/browse/HDFS-15711?focusedWorklogId=521919&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-521919
 ]

ASF GitHub Bot logged work on HDFS-15711:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 08/Dec/20 22:42
            Start Date: 08/Dec/20 22:42
    Worklog Time Spent: 10m 
      Work Description: jbrennan333 commented on a change in pull request #2521:
URL: https://github.com/apache/hadoop/pull/2521#discussion_r538846892



##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebApp.java
##########
@@ -123,6 +143,14 @@ public static HttpFSServerWebApp get() {
     return SERVER;
   }
 
+  /**
+   * gets the HttpFSServerMetrics instance.
+   * @return the HttpFSServerMetrics singleton.
+   */
+  public static HttpFSServerMetrics getMetrics() {
+    return metrics;
+  }
+
   /**
    * Returns HttpFSServer admin group.
    *

Review comment:
       I think you need to increment OpsCheckAccess() in FSAccess.execute(). 
   
   

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebApp.java
##########
@@ -110,9 +116,23 @@ public void init() throws ServerException {
   @Override
   public void destroy() {
     SERVER = null;
+    if (metrics != null) {
+      metrics.shutdown();
+    }
     super.destroy();
   }
 
+  public static void setMetrics(Configuration config) {

Review comment:
       Can you make this private?  It is only used by init() in this class.
   

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebApp.java
##########
@@ -110,9 +116,23 @@ public void init() throws ServerException {
   @Override
   public void destroy() {
     SERVER = null;
+    if (metrics != null) {
+      metrics.shutdown();
+    }
     super.destroy();
   }
 
+  public static void setMetrics(Configuration config) {
+    LOG.info("Initializing HttpFSServerMetrics");
+    HttpFSServerWebApp.metrics =

Review comment:
       Can't you just use metrics instead of HttpFSServerWebApp.metrics?  Same 
a few lines below.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
##########
@@ -542,8 +572,12 @@ private void createDirWithHttp(String dirname, String 
perms,
     conn.setRequestMethod("PUT");
     conn.connect();
     Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode());
+    Assert.assertEquals(1 + oldOpsMkdir,
+        HttpFSServerWebApp.get().getMetrics().getOpsMkdir());
   }
 
+
+

Review comment:
       Seems like extra blank lines were added by accident.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
##########
@@ -120,6 +122,25 @@
  */
 public class TestHttpFSServer extends HFSTestCase {
 
+  /**
+   * define metric getters for unit tests.
+   */
+  private static Callable<Long> defaultEntryMetricGetter = () -> 0L;
+  private static Callable<Long> defaultExitMetricGetter = () -> 1L;
+  private static HashMap<String, Callable<Long>> metricsGetter =
+      new HashMap<String, Callable<Long>>() {
+        {
+          put("LISTSTATUS",
+              () -> HttpFSServerWebApp.get().getMetrics().getOpsListing());
+          put("MKDIRS",
+              () -> HttpFSServerWebApp.get().getMetrics().getOpsMkdir());
+          put("GETFILESTATUS",
+              () -> HttpFSServerWebApp.get().getMetrics().getOpsStat());
+        }
+      };
+

Review comment:
       It's not clear to me why we need this metricsGetter.   Seems to add 
complexity with no added value.  Can't you just call the appropriate 
HttpFSServerWebApp.get().getMetrics().get* function wherever this is used?




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 521919)
    Time Spent: 50m  (was: 40m)

> Add Metrics to HttpFS Server
> ----------------------------
>
>                 Key: HDFS-15711
>                 URL: https://issues.apache.org/jira/browse/HDFS-15711
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>          Components: httpfs
>            Reporter: Ahmed Hussein
>            Assignee: Ahmed Hussein
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Currently HttpFS Server does not have any metrics.
> [~kihwal] has implemented serverMetrics for HttpFs on our internal grid.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to