You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/09/08 01:28:24 UTC

[GitHub] [hadoop] ZanderXu opened a new pull request, #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

ZanderXu opened a new pull request, #4871:
URL: https://github.com/apache/hadoop/pull/4871

   ### Description of PR
   Add a reQueue metric to RpcMetrics.java to quantify the number of RPCs reQueued. Because Observer NameNode will re-queue the rpc if the call processing should be postponed.
   There is no any metric to quantify the number of re-queue RPCs, so I think we should do it.
   
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] xkrogen commented on a diff in pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
xkrogen commented on code in PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#discussion_r968666305


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java:
##########
@@ -124,6 +130,42 @@ public static void shutDownCluster() throws IOException {
     }
   }
 
+  @Test
+  public void testObserverRequeue() throws Exception {
+    ScheduledExecutorService interruptor =
+        Executors.newScheduledThreadPool(1);
+
+    EditLogTailer observerEditlogTailer = dfsCluster.getNameNode(2)
+        .getNamesystem().getEditLogTailer();
+    RpcMetrics obRpcMetrics = ((NameNodeRpcServer)dfsCluster
+        .getNameNodeRpc(2)).getClientRpcServer().getRpcMetrics();
+
+    // Stop EditlogTailer of Observer NameNode.
+    observerEditlogTailer.stop();
+
+    long oldRequeueNum = obRpcMetrics.getRpcRequeueCalls();
+
+    ScheduledFuture<FileStatus> scheduledFuture = interruptor.schedule(
+        () -> {
+          Path tmpTestPath = new Path("/TestObserverRequeue");
+          dfs.create(tmpTestPath, (short)1).close();
+          assertSentTo(0);
+          // This operation will be blocked in ObserverNameNode
+          // until EditlogTailer tailed edits from journalNode.
+          FileStatus fileStatus = dfs.getFileStatus(tmpTestPath);
+          assertSentTo(2);
+          return fileStatus;
+        }, 0, TimeUnit.SECONDS);
+    Thread.sleep(1000);
+    observerEditlogTailer.doTailEdits();
+    FileStatus fileStatus = scheduledFuture.get(1000, TimeUnit.MILLISECONDS);

Review Comment:
   I would recommend a longer value like 10s -- 1s can easily get exceeded by some GC or other transient slowness event



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##########
@@ -6997,6 +6997,16 @@ public synchronized void verifyToken(DelegationTokenIdentifier identifier,
   public EditLogTailer getEditLogTailer() {
     return editLogTailer;
   }
+
+  @VisibleForTesting
+  public void startNewEditLogTailer(Configuration conf) throws IOException {
+    if (this.editLogTailer != null) {
+      this.editLogTailer.stop();
+    }
+
+    this.editLogTailer = new EditLogTailer(this, conf);
+    this.editLogTailer.start();
+  }

Review Comment:
   Can we just use existing methods `setEditLogTailerForTests()` and `getEditLogTailer()` instead of adding this new method?



##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java:
##########
@@ -124,6 +130,42 @@ public static void shutDownCluster() throws IOException {
     }
   }
 
+  @Test
+  public void testObserverRequeue() throws Exception {
+    ScheduledExecutorService interruptor =
+        Executors.newScheduledThreadPool(1);
+
+    EditLogTailer observerEditlogTailer = dfsCluster.getNameNode(2)
+        .getNamesystem().getEditLogTailer();
+    RpcMetrics obRpcMetrics = ((NameNodeRpcServer)dfsCluster
+        .getNameNodeRpc(2)).getClientRpcServer().getRpcMetrics();
+
+    // Stop EditlogTailer of Observer NameNode.
+    observerEditlogTailer.stop();
+
+    long oldRequeueNum = obRpcMetrics.getRpcRequeueCalls();
+
+    ScheduledFuture<FileStatus> scheduledFuture = interruptor.schedule(
+        () -> {
+          Path tmpTestPath = new Path("/TestObserverRequeue");
+          dfs.create(tmpTestPath, (short)1).close();
+          assertSentTo(0);
+          // This operation will be blocked in ObserverNameNode
+          // until EditlogTailer tailed edits from journalNode.
+          FileStatus fileStatus = dfs.getFileStatus(tmpTestPath);
+          assertSentTo(2);
+          return fileStatus;
+        }, 0, TimeUnit.SECONDS);
+    Thread.sleep(1000);

Review Comment:
   I guess you are trying to give the call time to be requeued?
   
   Instead of having a static `sleep()`, which can slow the test, I think using `GenericTestUtils.waitFor()` would be better. Something like:
   ```
   GenericTestUtils.waitFor(() -> obRpcMetrics.getRpcRequeueCalls() > oldRequeueNum, 50, 10000);
   FileStatus fileStatus = ...
   ```
   With a static sleep value, you have a tradeoff between a shorter sleep which can cause test flakiness (e.g. failure due to some temporary slowness) vs. long sleep which causes longer test runtime. A `waitFor` allows you to trade off by terminating as soon as the condition is met, but allowing for a longer maximum wait time to reduce flakiness.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1245543824

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 54s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 1 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 35s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  28m 25s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  25m 28s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |  22m  4s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   4m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 51s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 48s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 53s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   6m 45s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  27m  0s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 26s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 28s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  24m 39s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |  24m 39s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m  9s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  22m  9s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   4m 25s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/2/artifact/out/results-checkstyle-root.txt) |  root: The patch generated 2 new + 303 unchanged - 0 fixed = 305 total (was 303)  |
   | +1 :green_heart: |  mvnsite  |   3m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   3m  4s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 58s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   7m 19s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  27m 54s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 31s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 409m 53s |  |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 48s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 669m 45s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4871 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux c816a003dbef 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / bd38b8c38ad0967f3b72abe0b09e0c2394fc07a6 |
   | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/2/testReport/ |
   | Max. process+thread count | 2349 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1248058271

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 59s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 1 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  28m 51s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  25m 40s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |  22m 11s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   4m 28s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 47s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 48s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 51s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   6m 51s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  26m 50s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 27s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 27s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  24m 38s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |  24m 38s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 13s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  22m 13s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   4m 20s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/3/artifact/out/results-checkstyle-root.txt) |  root: The patch generated 1 new + 303 unchanged - 0 fixed = 304 total (was 303)  |
   | +1 :green_heart: |  mvnsite  |   3m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 42s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 55s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   6m 55s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  27m 20s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |  18m 31s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/3/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 379m 34s |  |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   3m 17s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 639m  2s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.http.TestHttpServer |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4871 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux d294f6262762 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / ce4297011ac789ab917fbd4db90f4da603d54a56 |
   | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/3/testReport/ |
   | Max. process+thread count | 2449 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] xkrogen commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
xkrogen commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1249592953

   The failing test in `TestHttpServer` was just added 2 days ago in PR #3972 and has been failing consistently since it was added (see [this comment](https://github.com/apache/hadoop/pull/3972#issuecomment-1249591906)). So we can ignore that failure.
   
   Merging to master, thanks for the contribution @ZanderXu !


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] xkrogen merged pull request #4871: HADOOP-18446. [SBN read] Add a re-queue metric to RpcMetrics to quantify the number of re-queued RPCs

Posted by GitBox <gi...@apache.org>.
xkrogen merged PR #4871:
URL: https://github.com/apache/hadoop/pull/4871


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1242831892

   @xkrogen Sir, can you help me review this patch? This metric is used to quantify the number of RPCs reQueued in Observer NameNode,


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] xkrogen commented on a diff in pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
xkrogen commented on code in PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#discussion_r971234834


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/metrics/RpcMetrics.java:
##########
@@ -344,6 +353,15 @@ public long getRpcSlowCalls() {
     return rpcSlowCalls.value();
   }
 
+  /**
+   * Returns the number of requeue calls;

Review Comment:
   Checkstyle complains:
   ```
   ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/metrics/RpcMetrics.java:192:  /**: First sentence should end with a period. [JavadocStyle]
   ```
   Can you change the semicolon to a period? `; -> .` 



##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java:
##########
@@ -156,14 +155,17 @@ public void testObserverRequeue() throws Exception {
           assertSentTo(2);
           return fileStatus;
         }, 0, TimeUnit.SECONDS);
-    Thread.sleep(1000);
-    observerEditlogTailer.doTailEdits();
-    FileStatus fileStatus = scheduledFuture.get(1000, TimeUnit.MILLISECONDS);
+
+    GenericTestUtils.waitFor(() -> obRpcMetrics.getRpcRequeueCalls() > oldRequeueNum,
+        50, 10000);
+
+    observerFsNS.getEditLogTailer().doTailEdits();
+    FileStatus fileStatus = scheduledFuture.get(10000, TimeUnit.MILLISECONDS);
     assertNotNull(fileStatus);
 
-    assertTrue(obRpcMetrics.getRpcRequeueCalls() > oldRequeueNum);
-    dfsCluster.getNameNode(2).getNamesystem()
-        .startNewEditLogTailer(dfsCluster.getConfiguration(2));
+    EditLogTailer editLogTailer = new EditLogTailer(observerFsNS, conf);
+    observerFsNS.setEditLogTailerForTests(editLogTailer);
+    editLogTailer.start();

Review Comment:
   Just realized we should do this in a try-finally since we are trying to clean up our modifications for other tests, e.g. right now if the `waitFor()` call fails then we won't reset the `editLogTailer` and other tests may fail as a result



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1248852885

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 56s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 1 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 53s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  32m 16s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  29m 26s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |  23m 18s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   4m 33s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 58s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 54s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m  9s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   7m 16s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  27m 46s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 30s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 36s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  27m 32s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |  27m 33s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m  6s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  23m  6s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  1s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   4m 24s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/artifact/out/results-checkstyle-root.txt) |  root: The patch generated 1 new + 303 unchanged - 0 fixed = 304 total (was 303)  |
   | +1 :green_heart: |  mvnsite  |   3m 56s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 51s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 52s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   7m  6s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  27m 45s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |  18m 27s | [/patch-unit-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt) |  hadoop-common in the patch passed.  |
   | -1 :x: |  unit  | 369m 14s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 27s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 641m 50s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.http.TestHttpServer |
   |   | hadoop.hdfs.server.namenode.ha.TestObserverNode |
   |   | hadoop.hdfs.server.mover.TestMover |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4871 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 33b6a9371a95 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / ce4297011ac789ab917fbd4db90f4da603d54a56 |
   | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/testReport/ |
   | Max. process+thread count | 3144 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/4/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #4871: HADOOP-18446. Add a re-queue metric to RpcMetrics.java to quantify the number of re-queue RPCs

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4871:
URL: https://github.com/apache/hadoop/pull/4871#issuecomment-1240634228

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m 16s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 1 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 41s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  28m 36s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  25m 51s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |  22m 16s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   4m 35s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 49s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 52s |  |  trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 54s |  |  trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   6m 50s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  26m 57s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 24s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 28s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  24m 56s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |  24m 56s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m  6s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  22m  6s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   4m 27s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/1/artifact/out/results-checkstyle-root.txt) |  root: The patch generated 2 new + 303 unchanged - 0 fixed = 305 total (was 303)  |
   | +1 :green_heart: |  mvnsite  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 43s |  |  the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   2m 53s |  |  the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   7m  1s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  27m 20s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 32s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 383m 58s |  |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 45s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 643m 13s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4871 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 465687b9ebe0 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 224a9451a11d44f7f7a04c6b0e4c306ca3bbfa91 |
   | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/1/testReport/ |
   | Max. process+thread count | 2070 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4871/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org