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 2020/04/11 14:05:01 UTC

[GitHub] [hadoop] brfrn169 opened a new pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

brfrn169 opened a new pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954
 
 
   ## NOTICE
   
   Please create an issue in ASF JIRA before opening a pull request,
   and you need to set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
   For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614281809
 
 
   @goiri I don't think the overhead is very large because building the lock report message happens only when the lock interval is more than the threshold. And to reduce the overhead, I use Supplier for the lazy lock report message building.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-615893744
 
 
   I did microbenchmarking with JMH. The code is as follows:
   https://gist.github.com/brfrn169/6a57175d934734b2a2c36652925ffdf6
   
   The results are as follows:
   
   Before applying the patch:
   ```
   Benchmark                         Mode  Cnt    Score   Error  Units
   FSNamesystemBenchmark.benchmark  thrpt   25  567.924 ± 3.184  ops/s
   ```
   
   After applying the patch:
   ```
   Benchmark                         Mode  Cnt    Score    Error  Units
   FSNamesystemBenchmark.benchmark  thrpt   25  573.884 ± 11.171  ops/s
   ```
   
   It looks like there isn't any performance regression after applying the patch.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407099108
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -253,10 +294,12 @@ public void writeUnlock(String opName, boolean suppressWriteLockReport) {
     LogAction logAction = LogThrottlingHelper.DO_NOT_LOG;
     if (needReport &&
         writeLockIntervalMs >= this.writeLockReportingThresholdMs) {
-      if (longestWriteLockHeldInfo.getIntervalMs() < writeLockIntervalMs) {
+      if (longestWriteLockHeldInfo.getIntervalMs() <= writeLockIntervalMs) {
+        String lockReportInfo = lockReportInfoSupplier != null ? " (" +
+            lockReportInfoSupplier.get() + ")" : "";
         longestWriteLockHeldInfo =
             new LockHeldInfo(currentTimeMs, writeLockIntervalMs,
-                StringUtils.getStackTrace(Thread.currentThread()));
+                StringUtils.getStackTrace(Thread.currentThread()), opName, lockReportInfo);
 
 Review comment:
   Extract the trace at least.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614352034
 
 
   @goiri Okay. Will do that.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614338796
 
 
   @brfrn169 I also believe that the Supplier logic should only be trigger if we log it.
   Can we do some benchmarking or profiling to verify this?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407099001
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -176,13 +181,23 @@ public void readUnlock(String opName) {
     final long readLockIntervalMs =
         TimeUnit.NANOSECONDS.toMillis(readLockIntervalNanos);
     if (needReport && readLockIntervalMs >= this.readLockReportingThresholdMs) {
-      LockHeldInfo localLockHeldInfo;
+      String lockReportInfo = null;
       do {
-        localLockHeldInfo = longestReadLockHeldInfo.get();
-      } while (localLockHeldInfo.getIntervalMs() - readLockIntervalMs < 0 &&
-          !longestReadLockHeldInfo.compareAndSet(localLockHeldInfo,
-              new LockHeldInfo(currentTimeMs, readLockIntervalMs,
-                  StringUtils.getStackTrace(Thread.currentThread()))));
+        LockHeldInfo localLockHeldInfo = longestReadLockHeldInfo.get();
+        if (localLockHeldInfo.getIntervalMs() <= readLockIntervalMs) {
+          if (lockReportInfo == null) {
+            lockReportInfo = lockReportInfoSupplier != null ? " (" +
+                lockReportInfoSupplier.get() + ")" : "";
+          }
+          if (longestReadLockHeldInfo.compareAndSet(localLockHeldInfo,
+            new LockHeldInfo(currentTimeMs, readLockIntervalMs,
 
 Review comment:
   Can we extract some of this? At least the stack trace.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612467222
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 12s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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 _ |
   | +1 :green_heart: |  mvninstall  |  21m 44s |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 48s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 13s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m 29s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   3m  2s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 44s |  hadoop-hdfs-project/hadoop-hdfs: The patch generated 81 new + 164 unchanged - 1 fixed = 245 total (was 165)  |
   | +1 :green_heart: |  mvnsite  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 33s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  the patch passed  |
   | -1 :x: |  findbugs  |   3m  5s |  hadoop-hdfs-project/hadoop-hdfs generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 110m 34s |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 37s |  The patch does not generate ASF License warnings.  |
   |  |   | 183m  0s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
   |  |  Possible null pointer dereference of effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:[line 7436] |
   |  |  Possible null pointer dereference of ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:[line 3207] |
   |  |  Possible null pointer dereference of res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:[line 3242] |
   | Failed junit tests | hadoop.hdfs.server.balancer.TestBalancer |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1954 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 652450b0e918 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 275c478 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/testReport/ |
   | Max. process+thread count | 2879 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/1/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri merged pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri merged pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612612443
 
 
   I fixed the checkstyle errors. And I don't think the findbugs errors are related to the patch.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-615947142
 
 
   Thanks @brfrn169 for the benchmark, I think this is safe enough.
   We've also been on this for a while so I guess nobody else has concerns.
   I'll go ahead and merge this.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407152458
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -159,10 +160,14 @@ public void readLockInterruptibly() throws InterruptedException {
   }
 
   public void readUnlock() {
-    readUnlock(OP_NAME_OTHER);
+    readUnlock(OP_NAME_OTHER, null);
   }
 
   public void readUnlock(String opName) {
+    readUnlock(opName, null);
+  }
+
+  public void readUnlock(String opName, Supplier<String> lockReportInfoSupplier) {
 
 Review comment:
   I use this Supplier in the following places:
   https://github.com/brfrn169/hadoop/blob/HDFS-15217/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java#L189-L190
   https://github.com/brfrn169/hadoop/blob/HDFS-15217/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java#L298-L299
   
   The reason why I use Supplier is that we don't always print the lock report, only when the lock interval is more than the threshold (**dfs.namenode.write-lock-reporting-threshold-ms** or **dfs.namenode.read-lock-reporting-threshold-ms**). We can do lazy building additional information with the **lockReportInfoSupplier**.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612644434
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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 _ |
   | +1 :green_heart: |  mvninstall  |  26m 48s |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   1m  0s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 35s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  21m 13s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   3m 53s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 49s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 54s |  hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 164 unchanged - 1 fixed = 164 total (was 165)  |
   | +1 :green_heart: |  mvnsite  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  17m 22s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  the patch passed  |
   | -1 :x: |  findbugs  |   3m 53s |  hadoop-hdfs-project/hadoop-hdfs generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 127m 55s |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 53s |  The patch does not generate ASF License warnings.  |
   |  |   | 215m 41s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
   |  |  Possible null pointer dereference of effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:[line 7443] |
   |  |  Possible null pointer dereference of ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:[line 3213] |
   |  |  Possible null pointer dereference of res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:[line 3248] |
   | Failed junit tests | hadoop.hdfs.TestRollingUpgrade |
   |   | hadoop.hdfs.server.namenode.TestFSNamesystemLock |
   |   | hadoop.hdfs.server.namenode.TestNameNodeMXBean |
   |   | hadoop.hdfs.server.namenode.ha.TestDFSUpgradeWithHA |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1954 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux d81063025720 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 8d49229 |
   | Default Java | 1.8.0_242 |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/3/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/3/testReport/ |
   | Max. process+thread count | 2841 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/3/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407236433
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemLockReport.java
 ##########
 @@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.security.PrivilegedExceptionAction;
+
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY;
+import static org.junit.Assert.assertTrue;
+
+public class TestFSNamesystemLockReport {
+
+  @FunctionalInterface
+  private interface SupplierWithException<T> {
+    T get() throws Exception;
+  }
+
+  @FunctionalInterface
+  private interface Procedure {
+    void invoke() throws Exception;
+  }
+
+  private Configuration conf;
+  private MiniDFSCluster cluster;
+  private FileSystem fs;
+  private UserGroupInformation userGroupInfo;
+  private GenericTestUtils.LogCapturer logs;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = new HdfsConfiguration();
+    conf.set(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, "hadoop");
+
+    // Make the lock report always shown
+    conf.setLong(DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY, 0);
+
+    cluster = new MiniDFSCluster.Builder(conf).numDataNodes(4).build();
+    fs = cluster.getFileSystem();
+
+    userGroupInfo = UserGroupInformation.createUserForTesting("bob",
+        new String[] {"hadoop"});
+
+    logs = GenericTestUtils.LogCapturer.captureLogs(FSNamesystem.LOG);
+    GenericTestUtils
+        .setLogLevel(LoggerFactory.getLogger(FSNamesystem.class.getName()),
+        org.slf4j.event.Level.INFO);
+  }
+
+  @After
+  public void cleanUp() throws Exception {
+    if (fs != null) {
+      fs.close();
+      fs = null;
+    }
+    if (cluster != null) {
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @Test
+  public void test() throws Exception {
+    FileSystem userfs = DFSTestUtil.getFileSystemAs(userGroupInfo, conf);
+
+    FSDataOutputStream os = testLockReport(() ->
+        userfs.create(new Path("/file")),
+        ".* by create \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=null," +
+        "perm=bob:hadoop:rw-r--r--\\) .*");
+    os.close();
+
+    FSDataInputStream is = testLockReport(() -> userfs.open(new Path("/file")),
+        ".* by open \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=null," +
+        "perm=null\\) .*");
+    is.close();
+
+    testLockReport(() ->
+        userfs.setPermission(new Path("/file"), new FsPermission(644)),
+        ".* by setPermission \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=null," +
+        "perm=bob:hadoop:-w----r-T\\) .*");
+
+    testLockReport(() -> userfs.setOwner(new Path("/file"), "alice", "group1"),
+        ".* by setOwner \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=null," +
+        "perm=alice:group1:-w----r-T\\) .*");
+
+    testLockReport(() -> userfs.listStatus(new Path("/")),
+        ".* by listStatus \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/,dst=null," +
+        "perm=null\\) .*");
+
+    testLockReport(() -> userfs.getFileStatus(new Path("/file")),
+        ".* by getfileinfo \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=null," +
+        "perm=null\\) .*");
+
+    testLockReport(() -> userfs.mkdirs(new Path("/dir")),
+        ".* by mkdirs \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/dir,dst=null," +
+        "perm=bob:hadoop:rwxr-xr-x\\) .*");
+
+    testLockReport(() -> userfs.rename(new Path("/file"), new Path("/file2")),
+        ".* by rename \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file,dst=/file2," +
+        "perm=alice:group1:-w----r-T\\) .*");
+
+    testLockReport(() -> userfs.delete(new Path("/file2"), false),
+        ".* by delete \\(ugi=bob \\(auth:SIMPLE\\)," +
+        "ip=/\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3},src=/file2,dst=null," +
+        "perm=null\\) .*");
+  }
+
+  private void testLockReport(Procedure procedure,
+      String expectedLockReportRegex) throws Exception {
+    logs.clearOutput();
+    userGroupInfo.doAs((PrivilegedExceptionAction<Void>) () -> {
+      procedure.invoke();
+      return null;
+    });
+
+    boolean matches = false;
+    for (String line : logs.getOutput().split(System.lineSeparator())) {
+      if (line.matches(expectedLockReportRegex)) {
+        matches = true;
+        break;
+      }
+    }
+    assertTrue(matches);
+  }
+
+  private <T> T testLockReport(SupplierWithException<T> supplier,
+      String expectedLockReportRegex) throws Exception {
+    logs.clearOutput();
+    T ret = userGroupInfo.doAs((PrivilegedExceptionAction<T>) supplier::get);
+
+    boolean matches = false;
 
 Review comment:
   This is the same in the previous method. Extract?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407099593
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -159,10 +160,14 @@ public void readLockInterruptibly() throws InterruptedException {
   }
 
   public void readUnlock() {
-    readUnlock(OP_NAME_OTHER);
+    readUnlock(OP_NAME_OTHER, null);
   }
 
   public void readUnlock(String opName) {
+    readUnlock(opName, null);
+  }
+
+  public void readUnlock(String opName, Supplier<String> lockReportInfoSupplier) {
 
 Review comment:
   I'm having a hard time finding who uses this Supplier. Where are we using this?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612778128
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 57s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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 _ |
   | +1 :green_heart: |  mvninstall  |  30m 13s |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 34s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 40s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  21m  4s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   4m 14s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m  8s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m 44s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 58s |  hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 164 unchanged - 1 fixed = 164 total (was 165)  |
   | +1 :green_heart: |  mvnsite  |   1m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  18m 18s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  the patch passed  |
   | -1 :x: |  findbugs  |   4m 18s |  hadoop-hdfs-project/hadoop-hdfs generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 136m 22s |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 59s |  The patch does not generate ASF License warnings.  |
   |  |   | 230m 50s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
   |  |  Possible null pointer dereference of effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:[line 7443] |
   |  |  Possible null pointer dereference of ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:[line 3213] |
   |  |  Possible null pointer dereference of res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:[line 3248] |
   | Failed junit tests | hadoop.hdfs.TestDFSStripedInputStreamWithRandomECPolicy |
   |   | hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting |
   |   | hadoop.hdfs.TestReadStripedFileWithDecodingDeletedData |
   |   | hadoop.hdfs.TestFileChecksum |
   |   | hadoop.hdfs.TestDecommission |
   |   | hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics |
   |   | hadoop.hdfs.server.balancer.TestBalancer |
   |   | hadoop.hdfs.TestErasureCodingPolicies |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1954 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux a81ab0443a22 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 8d49229 |
   | Default Java | 1.8.0_242 |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/4/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/4/testReport/ |
   | Max. process+thread count | 3464 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/4/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407294490
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemLockReport.java
 ##########
 @@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.security.PrivilegedExceptionAction;
+
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY;
+import static org.junit.Assert.assertTrue;
+
+public class TestFSNamesystemLockReport {
+
+  @FunctionalInterface
+  private interface SupplierWithException<T> {
+    T get() throws Exception;
+  }
+
+  @FunctionalInterface
+  private interface Procedure {
+    void invoke() throws Exception;
+  }
+
+  private Configuration conf;
+  private MiniDFSCluster cluster;
+  private FileSystem fs;
+  private UserGroupInformation userGroupInfo;
+  private GenericTestUtils.LogCapturer logs;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = new HdfsConfiguration();
+    conf.set(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, "hadoop");
+
+    // Make the lock report always shown
+    conf.setLong(DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY, 0);
+
+    cluster = new MiniDFSCluster.Builder(conf).numDataNodes(4).build();
+    fs = cluster.getFileSystem();
+
+    userGroupInfo = UserGroupInformation.createUserForTesting("bob",
+        new String[] {"hadoop"});
+
+    logs = GenericTestUtils.LogCapturer.captureLogs(FSNamesystem.LOG);
+    GenericTestUtils
+        .setLogLevel(LoggerFactory.getLogger(FSNamesystem.class.getName()),
+        org.slf4j.event.Level.INFO);
+  }
+
+  @After
+  public void cleanUp() throws Exception {
+    if (fs != null) {
+      fs.close();
+      fs = null;
+    }
+    if (cluster != null) {
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @Test
+  public void test() throws Exception {
+    FileSystem userfs = DFSTestUtil.getFileSystemAs(userGroupInfo, conf);
+
+    FSDataOutputStream os = testLockReport(() ->
 
 Review comment:
   I posted the example in the Jira:
   https://issues.apache.org/jira/browse/HDFS-15217
   
   Do we still need to put it here?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-613191918
 
 
   @goiri I added comments in the test. 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407098774
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -106,8 +107,8 @@ public Long initialValue() {
    * lock was held since the last report.
    */
   private final AtomicReference<LockHeldInfo> longestReadLockHeldInfo =
-      new AtomicReference<>(new LockHeldInfo(0, 0, null));
-  private LockHeldInfo longestWriteLockHeldInfo = new LockHeldInfo(0, 0, null);
+      new AtomicReference<>(new LockHeldInfo(0, 0, null, null, null));
+  private LockHeldInfo longestWriteLockHeldInfo = new LockHeldInfo(0, 0, null, null, null);
 
 Review comment:
   Can we just keep the old constructor with the old parameters that pass the null to the new one instead of changing everywhere?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407605437
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemLockReport.java
 ##########
 @@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.security.PrivilegedExceptionAction;
+
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY;
+import static org.junit.Assert.assertTrue;
+
+public class TestFSNamesystemLockReport {
+
+  @FunctionalInterface
+  private interface SupplierWithException<T> {
+    T get() throws Exception;
+  }
+
+  @FunctionalInterface
+  private interface Procedure {
+    void invoke() throws Exception;
+  }
+
+  private Configuration conf;
+  private MiniDFSCluster cluster;
+  private FileSystem fs;
+  private UserGroupInformation userGroupInfo;
+  private GenericTestUtils.LogCapturer logs;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = new HdfsConfiguration();
+    conf.set(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, "hadoop");
+
+    // Make the lock report always shown
+    conf.setLong(DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY, 0);
+
+    cluster = new MiniDFSCluster.Builder(conf).numDataNodes(4).build();
+    fs = cluster.getFileSystem();
+
+    userGroupInfo = UserGroupInformation.createUserForTesting("bob",
+        new String[] {"hadoop"});
+
+    logs = GenericTestUtils.LogCapturer.captureLogs(FSNamesystem.LOG);
+    GenericTestUtils
+        .setLogLevel(LoggerFactory.getLogger(FSNamesystem.class.getName()),
+        org.slf4j.event.Level.INFO);
+  }
+
+  @After
+  public void cleanUp() throws Exception {
+    if (fs != null) {
+      fs.close();
+      fs = null;
+    }
+    if (cluster != null) {
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @Test
+  public void test() throws Exception {
+    FileSystem userfs = DFSTestUtil.getFileSystemAs(userGroupInfo, conf);
+
+    FSDataOutputStream os = testLockReport(() ->
 
 Review comment:
   I think it would help for each of the lines we check what would be an example line.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 edited a comment on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-615893744
 
 
   I did microbenchmarking with JMH. The microbenchmarking  code is as follows:
   https://gist.github.com/brfrn169/6a57175d934734b2a2c36652925ffdf6
   
   The results are as follows:
   
   Before applying the patch:
   ```
   Benchmark                         Mode  Cnt    Score   Error  Units
   FSNamesystemBenchmark.benchmark  thrpt   25  567.924 ± 3.184  ops/s
   ```
   
   After applying the patch:
   ```
   Benchmark                         Mode  Cnt    Score    Error  Units
   FSNamesystemBenchmark.benchmark  thrpt   25  573.884 ± 11.171  ops/s
   ```
   
   It looks like there isn't any performance regression after applying the patch.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407235813
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemLockReport.java
 ##########
 @@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.security.PrivilegedExceptionAction;
+
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY;
+import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY;
+import static org.junit.Assert.assertTrue;
+
+public class TestFSNamesystemLockReport {
+
+  @FunctionalInterface
+  private interface SupplierWithException<T> {
+    T get() throws Exception;
+  }
+
+  @FunctionalInterface
+  private interface Procedure {
+    void invoke() throws Exception;
+  }
+
+  private Configuration conf;
+  private MiniDFSCluster cluster;
+  private FileSystem fs;
+  private UserGroupInformation userGroupInfo;
+  private GenericTestUtils.LogCapturer logs;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = new HdfsConfiguration();
+    conf.set(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, "hadoop");
+
+    // Make the lock report always shown
+    conf.setLong(DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, 0);
+    conf.setLong(DFS_LOCK_SUPPRESS_WARNING_INTERVAL_KEY, 0);
+
+    cluster = new MiniDFSCluster.Builder(conf).numDataNodes(4).build();
+    fs = cluster.getFileSystem();
+
+    userGroupInfo = UserGroupInformation.createUserForTesting("bob",
+        new String[] {"hadoop"});
+
+    logs = GenericTestUtils.LogCapturer.captureLogs(FSNamesystem.LOG);
+    GenericTestUtils
+        .setLogLevel(LoggerFactory.getLogger(FSNamesystem.class.getName()),
+        org.slf4j.event.Level.INFO);
+  }
+
+  @After
+  public void cleanUp() throws Exception {
+    if (fs != null) {
+      fs.close();
+      fs = null;
+    }
+    if (cluster != null) {
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @Test
+  public void test() throws Exception {
+    FileSystem userfs = DFSTestUtil.getFileSystemAs(userGroupInfo, conf);
+
+    FSDataOutputStream os = testLockReport(() ->
 
 Review comment:
   Add a comment with the example of the actual line.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-613238448
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 17s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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 _ |
   | +1 :green_heart: |  mvninstall  |  25m 36s |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 54s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 28s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  18m 19s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 46s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   3m 26s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 25s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 47s |  hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 164 unchanged - 1 fixed = 164 total (was 165)  |
   | +1 :green_heart: |  mvnsite  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 31s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  the patch passed  |
   | -1 :x: |  findbugs  |   3m 28s |  hadoop-hdfs-project/hadoop-hdfs generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 118m 25s |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 40s |  The patch does not generate ASF License warnings.  |
   |  |   | 197m 51s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
   |  |  Possible null pointer dereference of effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:[line 7443] |
   |  |  Possible null pointer dereference of ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:[line 3213] |
   |  |  Possible null pointer dereference of res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:[line 3248] |
   | Failed junit tests | hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes |
   |   | hadoop.hdfs.server.datanode.TestBPOfferService |
   |   | hadoop.hdfs.server.datanode.TestNNHandlesBlockReportPerStorage |
   |   | hadoop.hdfs.server.namenode.ha.TestEditLogTailer |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1954 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 756ef23e6abd 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 3edbe87 |
   | Default Java | 1.8.0_242 |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/5/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/5/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/5/testReport/ |
   | Max. process+thread count | 3178 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/5/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614352034
 
 
   @brfrn169 Okay. Will do that.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612727480
 
 
   It looks like extracting the getStackTrace method broke the test of TestFSNamesystemLock. I reverted 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-612589687
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 46s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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 _ |
   | +1 :green_heart: |  mvninstall  |  19m 46s |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 53s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 18s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 50s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 46s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   2m 54s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m 53s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  1s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 44s |  hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 164 unchanged - 1 fixed = 166 total (was 165)  |
   | +1 :green_heart: |  mvnsite  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m  2s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  the patch passed  |
   | -1 :x: |  findbugs  |   2m 59s |  hadoop-hdfs-project/hadoop-hdfs generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  97m 22s |  hadoop-hdfs in the patch passed.  |
   | +0 :ok: |  asflicense  |   0m 16s |  ASF License check generated no output?  |
   |  |   | 164m  2s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
   |  |  Possible null pointer dereference of effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:effectiveDirective in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.addCacheDirective(CacheDirectiveInfo, EnumSet, boolean)  Dereferenced at FSNamesystem.java:[line 7442] |
   |  |  Possible null pointer dereference of ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:ret in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean)  Dereferenced at FSNamesystem.java:[line 3213] |
   |  |  Possible null pointer dereference of res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:res in org.apache.hadoop.hdfs.server.namenode.FSNamesystem.renameTo(String, String, boolean, Options$Rename[])  Dereferenced at FSNamesystem.java:[line 3248] |
   | Failed junit tests | hadoop.hdfs.TestDFSStripedOutputStreamWithFailureWithRandomECPolicy |
   |   | hadoop.hdfs.TestDecommissionWithStriped |
   |   | hadoop.hdfs.TestSetrepIncreasing |
   |   | hadoop.hdfs.TestFileChecksumCompositeCrc |
   |   | hadoop.hdfs.TestMaintenanceState |
   |   | hadoop.hdfs.TestErasureCodingPolicyWithSnapshot |
   |   | hadoop.hdfs.tools.TestECAdmin |
   |   | hadoop.hdfs.TestQuotaAllowOwner |
   |   | hadoop.hdfs.TestDFSUpgrade |
   |   | hadoop.hdfs.TestHFlush |
   |   | hadoop.hdfs.tools.TestViewFSStoragePolicyCommands |
   |   | hadoop.hdfs.TestDecommissionWithBackoffMonitor |
   |   | hadoop.hdfs.protocol.datatransfer.sasl.TestSaslDataTransfer |
   |   | hadoop.hdfs.TestFileCorruption |
   |   | hadoop.hdfs.TestErasureCodingExerciseAPIs |
   |   | hadoop.hdfs.TestReadStripedFileWithMissingBlocks |
   |   | hadoop.hdfs.TestReadStripedFileWithDecodingDeletedData |
   |   | hadoop.hdfs.TestReadStripedFileWithDNFailure |
   |   | hadoop.hdfs.TestClientProtocolForPipelineRecovery |
   |   | hadoop.hdfs.tools.TestDFSAdmin |
   |   | hadoop.hdfs.TestDecommission |
   |   | hadoop.hdfs.tools.TestStoragePolicySatisfyAdminCommands |
   |   | hadoop.hdfs.TestDFSClientRetries |
   |   | hadoop.hdfs.TestFileStatus |
   |   | hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewerWithStripedBlocks |
   |   | hadoop.hdfs.TestDFSInotifyEventInputStream |
   |   | hadoop.hdfs.TestFileChecksum |
   |   | hadoop.hdfs.TestAppendSnapshotTruncate |
   |   | hadoop.hdfs.TestStripedFileAppend |
   |   | hadoop.hdfs.TestRestartDFS |
   |   | hadoop.hdfs.TestGetBlocks |
   |   | hadoop.hdfs.TestDistributedFileSystemWithECFileWithRandomECPolicy |
   |   | hadoop.hdfs.security.token.block.TestBlockToken |
   |   | hadoop.hdfs.TestFileCreation |
   |   | hadoop.hdfs.security.TestDelegationToken |
   |   | hadoop.hdfs.TestDFSStartupVersions |
   |   | hadoop.hdfs.TestClientReportBadBlock |
   |   | hadoop.cli.TestHDFSCLI |
   |   | hadoop.hdfs.TestModTime |
   |   | hadoop.hdfs.TestErasureCodingPoliciesWithRandomECPolicy |
   |   | hadoop.hdfs.TestDFSStripedOutputStream |
   |   | hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer |
   |   | hadoop.hdfs.TestBlocksScheduledCounter |
   |   | hadoop.hdfs.tools.TestDebugAdmin |
   |   | hadoop.hdfs.TestDFSUpgradeFromImage |
   |   | hadoop.hdfs.TestErasureCodingPolicyWithSnapshotWithRandomECPolicy |
   |   | hadoop.hdfs.TestLeaseRecoveryStriped |
   |   | hadoop.hdfs.TestBlockTokenWrappingQOP |
   |   | hadoop.hdfs.web.TestWebHDFS |
   |   | hadoop.hdfs.TestDFSStripedOutputStreamWithFailure |
   |   | hadoop.hdfs.tools.TestDFSAdminWithHA |
   |   | hadoop.hdfs.TestErasureCodingPolicies |
   |   | hadoop.hdfs.security.TestDelegationTokenForProxyUser |
   |   | hadoop.hdfs.TestDFSStripedInputStream |
   |   | hadoop.hdfs.TestEncryptionZonesWithHA |
   |   | hadoop.hdfs.TestRollingUpgrade |
   |   | hadoop.hdfs.TestDatanodeRegistration |
   |   | hadoop.hdfs.TestWriteReadStripedFile |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1954 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 1b3d6d7aab73 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 8d49229 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/testReport/ |
   | Max. process+thread count | 2028 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1954/2/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614281809
 
 
   @goiri I don't think the overhead is not large because building the lock report message happens only when the lock interval is more than the threshold. And to reduce the overhead, I use Supplier for the lazy lock report message building.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on issue #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#issuecomment-614267348
 
 
   @brfrn169 thanks for updating.
   One concern I have is the added overhead.
   Specially, the overhead within the write lock.
   Any thoughts there?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r409032699
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
 ##########
 @@ -3204,11 +3235,12 @@ void renameTo(final String src, final String dst,
         res = FSDirRenameOp.renameToInt(dir, pc, src, dst, logRetryCache,
             options);
       } finally {
-        writeUnlock(operationName);
+        FileStatus status = res != null ? res.auditStat : null;
+        writeUnlock(operationName,
+            getLockReportInfoSupplier(src, dst, status));
       }
     } catch (AccessControlException e) {
-      logAuditEvent(false, operationName + " (options=" +
-          Arrays.toString(options) + ")", src, dst, null);
+      logAuditEvent(false, operationName, src, dst, null);
 
 Review comment:
   I think you need to rebase, right?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407141488
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -106,8 +107,8 @@ public Long initialValue() {
    * lock was held since the last report.
    */
   private final AtomicReference<LockHeldInfo> longestReadLockHeldInfo =
-      new AtomicReference<>(new LockHeldInfo(0, 0, null));
-  private LockHeldInfo longestWriteLockHeldInfo = new LockHeldInfo(0, 0, null);
+      new AtomicReference<>(new LockHeldInfo(0, 0, null, null, null));
+  private LockHeldInfo longestWriteLockHeldInfo = new LockHeldInfo(0, 0, null, null, null);
 
 Review comment:
   Maybe it's better to have default constructor (without any parameters) of LockHeldInfo for those initial instances? I will try to do that.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r409103492
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
 ##########
 @@ -3204,11 +3235,12 @@ void renameTo(final String src, final String dst,
         res = FSDirRenameOp.renameToInt(dir, pc, src, dst, logRetryCache,
             options);
       } finally {
-        writeUnlock(operationName);
+        FileStatus status = res != null ? res.auditStat : null;
+        writeUnlock(operationName,
+            getLockReportInfoSupplier(src, dst, status));
       }
     } catch (AccessControlException e) {
-      logAuditEvent(false, operationName + " (options=" +
-          Arrays.toString(options) + ")", src, dst, null);
+      logAuditEvent(false, operationName, src, dst, null);
 
 Review comment:
   Thank you for pointing out this. I modified the patch for the review.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


[GitHub] [hadoop] goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1954: HDFS-15217 Add more information to longest write/read lock held log
URL: https://github.com/apache/hadoop/pull/1954#discussion_r407098949
 
 

 ##########
 File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
 ##########
 @@ -176,13 +181,23 @@ public void readUnlock(String opName) {
     final long readLockIntervalMs =
         TimeUnit.NANOSECONDS.toMillis(readLockIntervalNanos);
     if (needReport && readLockIntervalMs >= this.readLockReportingThresholdMs) {
-      LockHeldInfo localLockHeldInfo;
+      String lockReportInfo = null;
       do {
-        localLockHeldInfo = longestReadLockHeldInfo.get();
-      } while (localLockHeldInfo.getIntervalMs() - readLockIntervalMs < 0 &&
-          !longestReadLockHeldInfo.compareAndSet(localLockHeldInfo,
-              new LockHeldInfo(currentTimeMs, readLockIntervalMs,
-                  StringUtils.getStackTrace(Thread.currentThread()))));
+        LockHeldInfo localLockHeldInfo = longestReadLockHeldInfo.get();
+        if (localLockHeldInfo.getIntervalMs() <= readLockIntervalMs) {
+          if (lockReportInfo == null) {
+            lockReportInfo = lockReportInfoSupplier != null ? " (" +
+                lockReportInfoSupplier.get() + ")" : "";
+          }
+          if (longestReadLockHeldInfo.compareAndSet(localLockHeldInfo,
+            new LockHeldInfo(currentTimeMs, readLockIntervalMs,
+              StringUtils.getStackTrace(Thread.currentThread()), opName, lockReportInfo))) {
+            break;
+          }
+        } else {
+          break;
+        }
+      } while (true);
 
 Review comment:
   As we are touching this, can we make this into a regular:
   boolean done = false;
   while (!done) {
    ...
    } else {
     done = true;
    }
   }
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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