Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2025-01-07 Thread via GitHub


kokon191 commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2576604154

   Failed tests seem to be unrelated to FGL. In particular, 
hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped failure was 
caused by HDFS-17080 and 
hadoop.hdfs.server.federation.store.driver.TestStateStoreFileSystem by 
HDFS-17496


-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2025-01-07 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2575025799

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 42s |  |  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.  
|
   | +0 :ok: |  xmllint  |   0m  1s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   9m 18s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  30m 41s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 43s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  14m 56s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 18s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 10s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 59s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 24s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 47s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  33m  1s |  |  branch has no errors 
when building and testing our client artifacts.  |
   | -0 :warning: |  patch  |  33m 27s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 32s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 55s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 10s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  16m 10s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 16s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  15m 16s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  1s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 21s |  |  root: The patch generated 
0 new + 1233 unchanged - 16 fixed = 1233 total (was 1249)  |
   | +1 :green_heart: |  mvnsite  |   3m 11s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 18s |  |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 0 new + 3113 
unchanged - 4 fixed = 3113 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  |  hadoop-hdfs-rbf in the patch 
passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  |  hadoop-fs2img in the patch 
passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.  |
   | +1 :green_heart: |  javadoc  |   3m 15s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 21s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  33m 51s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 230m 47s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/9/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | -1 :x: |  unit  |  30m 59s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/9/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt)
 |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 59s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m 14s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 483m 32s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | 
hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped |
   |   | hadoop.hdfs.server.federation.store.driver.TestStateStoreFileSystem |
   
   
   | Subsystem | Report/Notes |
   |--

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-31 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2566382405

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 31s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  2s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +0 :ok: |  xmllint  |   0m  0s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   7m 24s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  30m 20s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 31s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  14m 42s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 20s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 12s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   3m  3s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 22s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 44s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  33m  8s |  |  branch has no errors 
when building and testing our client artifacts.  |
   | -0 :warning: |  patch  |  33m 34s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 32s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 56s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 45s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  15m 45s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 13s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  15m 13s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 20s |  |  root: The patch generated 
0 new + 1233 unchanged - 16 fixed = 1233 total (was 1249)  |
   | +1 :green_heart: |  mvnsite  |   3m  9s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 19s |  |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 0 new + 3113 
unchanged - 4 fixed = 3113 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   0m 54s |  |  hadoop-hdfs-rbf in the patch 
passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  |  hadoop-fs2img in the patch 
passed with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.  |
   | +1 :green_heart: |  javadoc  |   3m 28s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 19s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  33m 30s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 224m 24s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/8/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  unit  |  30m 22s |  |  hadoop-hdfs-rbf in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   0m 59s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m 13s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 473m 10s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | hadoop.hdfs.TestDecommissionWithBackoffMonitor |
   |   | hadoop.hdfs.TestDecommission |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.47 ServerAPI=1.47 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/8/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   |

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-28 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2564408198

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 31s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  2s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +0 :ok: |  xmllint  |   0m  0s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m  1s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  31m 24s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  17m  7s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  15m  0s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 22s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 53s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 48s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 11s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 49s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  33m 57s |  |  branch has no errors 
when building and testing our client artifacts.  |
   | -0 :warning: |  patch  |  34m 22s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 31s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 54s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  17m 18s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  17m 18s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m  6s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  15m  6s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 10s |  |  root: The patch generated 
0 new + 1233 unchanged - 16 fixed = 1233 total (was 1249)  |
   | +1 :green_heart: |  mvnsite  |   2m 49s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 17s | 
[/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/7/artifact/out/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt)
 |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 2 new + 3115 
unchanged - 2 fixed = 3117 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   3m  5s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 39s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  39m 11s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 225m  4s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/7/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  unit  |  30m 19s |  |  hadoop-hdfs-rbf in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   0m 59s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m 14s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 480m 18s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | hadoop.hdfs.TestDecommissionWithBackoffMonitor |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.47 ServerAPI=1.47 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/7/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Optional Tests | dupname asflicense com

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-26 Thread via GitHub


ZanderXu commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1897862361


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/fgl/FineGrainedFSNamesystemLock.java:
##
@@ -0,0 +1,300 @@
+/**
+ * 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.fgl;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdfs.server.namenode.FSNamesystemLock;
+import org.apache.hadoop.metrics2.lib.MutableRatesWithAggregation;
+
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
+
+/**
+ * Splitting the global FSN lock into FSLock and BMLock.
+ * FSLock is used to protect directory tree-related operations.
+ * BMLock is used to protect block-related and dn-related operations.
+ * The lock order should be: FSLock,BMLock.
+ */
+public class FineGrainedFSNamesystemLock implements FSNLockManager {
+  private final FSNamesystemLock fsLock;
+  private final FSNamesystemLock bmLock;
+
+  public FineGrainedFSNamesystemLock(Configuration conf, 
MutableRatesWithAggregation aggregation) {
+this.fsLock = new FSNamesystemLock(conf, "FS", aggregation);
+this.bmLock = new FSNamesystemLock(conf, "BM", aggregation);
+  }
+
+  @Override
+  public void readLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLock();
+  this.bmLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLock();
+}
+  }
+
+  public void readLockInterruptibly(FSNamesystemLockMode lockMode) throws 
InterruptedException  {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLockInterruptibly();
+  try {
+this.bmLock.readLockInterruptibly();
+  } catch (InterruptedException e) {
+// The held FSLock should be released if the current thread is 
interrupted
+// while acquiring the BMLock.
+this.fsLock.readUnlock("BMReadLockInterruptiblyFailed");
+throw e;
+  }
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLockInterruptibly();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLockInterruptibly();
+}
+  }
+
+  @Override
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName);
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName);
+}
+  }
+
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName,
+  Supplier lockReportInfoSupplier) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+}
+  }
+
+  @Override
+  public void writeLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.writeLock();
+  this.bmLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.writeLock();
+}
+  }
+
+  @Override
+  public void writeUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.writeUnlock(opName);
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.write

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-25 Thread via GitHub


ZanderXu commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2562240142

   > I know this PR is phase 1 work: split global lock into fslock and bmlock. 
We should ensure the lock order: bmlock cannot occur in front of fslock. Can we 
add some restrictions or check mechanisms to avoid getting a bmlock before 
fslock?
   
   Thanks, @zhengchenyu . This is a good idea. 
   The check mechanism is theoretically feasible (verifying whether the BMLock 
is already held when acquiring the FSLock). However, due to lock reentry, this 
approach introduces complexity and negatively impacts the performance.
   
   Let's briefly consider some common scenarios:
   1. A thread acquires the FS read lock for the first time.
   2. A thread acquires the FS write lock for the first time.
   3. A thread holding the Global read lock reenters the FS read lock (already 
holding the BM read lock).
   4. A thread holding the Global write lock reenters the FS read lock (already 
holding the BM write lock).
   5. A thread holding the Global write lock reenters the FS write lock 
(already holding the BM write lock).
   
   In scenarios 3, 4, and 5, the BM lock already held is normal when acquiring 
FS lock. However, in scenarios 1 and 2, the BM lock already held is abnormal 
when acquiring FS lock. 
   
   Indeed, lock reentry can also be checked, but I think it’s unnecessary as 
it’s overly complicated. We can use other methods to detect deadlock issues.


-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-25 Thread via GitHub


ZanderXu commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1897678303


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/fgl/FineGrainedFSNamesystemLock.java:
##
@@ -0,0 +1,300 @@
+/**
+ * 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.fgl;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdfs.server.namenode.FSNamesystemLock;
+import org.apache.hadoop.metrics2.lib.MutableRatesWithAggregation;
+
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
+
+/**
+ * Splitting the global FSN lock into FSLock and BMLock.
+ * FSLock is used to protect directory tree-related operations.
+ * BMLock is used to protect block-related and dn-related operations.
+ * The lock order should be: FSLock,BMLock.
+ */
+public class FineGrainedFSNamesystemLock implements FSNLockManager {
+  private final FSNamesystemLock fsLock;
+  private final FSNamesystemLock bmLock;
+
+  public FineGrainedFSNamesystemLock(Configuration conf, 
MutableRatesWithAggregation aggregation) {
+this.fsLock = new FSNamesystemLock(conf, "FS", aggregation);
+this.bmLock = new FSNamesystemLock(conf, "BM", aggregation);
+  }
+
+  @Override
+  public void readLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLock();
+  this.bmLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLock();
+}
+  }
+
+  public void readLockInterruptibly(FSNamesystemLockMode lockMode) throws 
InterruptedException  {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLockInterruptibly();
+  try {
+this.bmLock.readLockInterruptibly();
+  } catch (InterruptedException e) {
+// The held FSLock should be released if the current thread is 
interrupted
+// while acquiring the BMLock.
+this.fsLock.readUnlock("BMReadLockInterruptiblyFailed");
+throw e;
+  }
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLockInterruptibly();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLockInterruptibly();
+}
+  }
+
+  @Override
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName);
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName);
+}
+  }
+
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName,
+  Supplier lockReportInfoSupplier) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+}
+  }
+
+  @Override
+  public void writeLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.writeLock();
+  this.bmLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.writeLock();
+}
+  }
+
+  @Override
+  public void writeUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.writeUnlock(opName);
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.write

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-25 Thread via GitHub


ZanderXu commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1897589520


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -2058,7 +2080,7 @@ BatchedListEntries listOpenFiles(long 
prevId,
 
   public BatchedListEntries getFilesBlockingDecom(long prevId,
   String path) {
-assert hasReadLock();
+assert hasReadLock(RwLockMode.FS);

Review Comment:
   Thanks @hfutatzhanghb for your review. 
   Both `dataNode.getLeavingServiceStatus().getOpenFiles()` and 
`blockManager.getDatanodeManager().getDatanodes())` are already thread-safe, so 
the BMLock is unnecessary 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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-25 Thread via GitHub


hfutatzhanghb commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1897587322


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -2031,7 +2053,7 @@ BatchedListEntries listOpenFiles(long 
prevId,
 BatchedListEntries batchedListEntries;
 String normalizedPath = new Path(path).toString(); // normalize path.
 try {
-  readLock();
+  readLock(RwLockMode.FS);

Review Comment:
   Also use GLOBAL mode here, because it invoke getFilesBlockingDecom method.



##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -2058,7 +2080,7 @@ BatchedListEntries listOpenFiles(long 
prevId,
 
   public BatchedListEntries getFilesBlockingDecom(long prevId,
   String path) {
-assert hasReadLock();
+assert hasReadLock(RwLockMode.FS);

Review Comment:
   I think we should use GLOBAL mode here, because below codes contain iterate 
`blockManager.getDatanodeManager().getDatanodes()`



-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-25 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2561959139

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 33s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  2s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +0 :ok: |  xmllint  |   0m  0s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 54s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  29m 50s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 23s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  15m  7s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 20s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 13s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   3m  0s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 27s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 42s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  33m  2s |  |  branch has no errors 
when building and testing our client artifacts.  |
   | -0 :warning: |  patch  |  33m 28s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 32s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 54s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 51s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  15m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m  6s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  15m  6s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 15s |  |  root: The patch generated 
0 new + 1233 unchanged - 16 fixed = 1233 total (was 1249)  |
   | +1 :green_heart: |  mvnsite  |   3m 11s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 19s | 
[/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/6/artifact/out/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt)
 |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 13 new + 3117 
unchanged - 0 fixed = 3130 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   3m 24s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 15s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  33m  6s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  | 227m 34s |  |  hadoop-hdfs in the patch 
passed.  |
   | +1 :green_heart: |  unit  |  30m 15s |  |  hadoop-hdfs-rbf in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   0m 59s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m 12s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 474m  1s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.47 ServerAPI=1.47 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/6/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint 
markdownlint |
   | uname | Linux 77e0fea14a86 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 
20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personalit

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-24 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2561278567

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 33s |  |  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.  
|
   | +0 :ok: |  xmllint  |   0m  1s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 57s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  29m 58s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 25s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  14m 46s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 22s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 12s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 54s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 21s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 45s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  33m 16s |  |  branch has no errors 
when building and testing our client artifacts.  |
   | -0 :warning: |  patch  |  33m 42s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 32s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 55s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 41s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  15m 41s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  14m 55s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  14m 55s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 19s |  |  root: The patch generated 
0 new + 1233 unchanged - 16 fixed = 1233 total (was 1249)  |
   | +1 :green_heart: |  mvnsite  |   3m 10s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 19s | 
[/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/5/artifact/out/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt)
 |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 13 new + 3117 
unchanged - 0 fixed = 3130 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   3m 20s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 18s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  33m 40s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 222m 50s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/5/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  unit  |  30m 16s |  |  hadoop-hdfs-rbf in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   0m 59s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m 12s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 469m 13s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | hadoop.hdfs.TestDecommission |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.47 ServerAPI=1.47 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/5/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Optional Tests | dupname asflicense compile javac javado

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-12-19 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2553294502

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 31s |  |  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.  
|
   | +0 :ok: |  xmllint  |   0m  1s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint 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 49 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   7m 32s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  31m 51s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 22s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  compile  |  15m  1s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  checkstyle  |   4m 26s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 11s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   3m  2s |  |  trunk passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   3m 26s |  |  trunk passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   5m 52s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  35m  5s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 33s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 59s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 42s |  |  the patch passed with JDK 
Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04  |
   | +1 :green_heart: |  javac  |  15m 42s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  14m 52s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  javac  |  14m 52s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   4m 23s | 
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/4/artifact/out/results-checkstyle-root.txt)
 |  root: The patch generated 7 new + 1233 unchanged - 16 fixed = 1240 total 
(was 1249)  |
   | +1 :green_heart: |  mvnsite  |   3m  8s |  |  the patch passed  |
   | -1 :x: |  javadoc  |   1m 19s | 
[/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/4/artifact/out/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04.txt)
 |  
hadoop-hdfs-project_hadoop-hdfs-jdkUbuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 
with JDK Ubuntu-11.0.25+9-post-Ubuntu-1ubuntu120.04 generated 13 new + 3117 
unchanged - 0 fixed = 3130 total (was 3117)  |
   | +1 :green_heart: |  javadoc  |   3m 21s |  |  the patch passed with JDK 
Private Build-1.8.0_432-8u432-ga~us1-0ubuntu2~20.04-ga  |
   | +1 :green_heart: |  spotbugs  |   6m 15s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  33m 28s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 220m 51s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | -1 :x: |  unit  |  30m  0s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt)
 |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  unit  |   0m 53s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m  6s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 472m 40s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | hadoop.hdfs.TestDecommissionWithBackoffMonitor |
   |   | hadoop.hdfs.TestDecommission |
   |   | hadoop.hdfs.server.federation.router.TestRouterClientRejectOverload |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | 

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-05-31 Thread via GitHub


zhengchenyu commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2141744912

   I know this PR is phase 1 work: split global lock into fslock and bmlock.
   We should ensure the lock order: bmlock cannot occur in front of fslock. Can 
we add some restrictions or check mechanisms to avoid getting a bmlock before 
fslock?


-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-05-31 Thread via GitHub


zhengchenyu commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1622166340


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/fgl/FineGrainedFSNamesystemLock.java:
##
@@ -0,0 +1,300 @@
+/**
+ * 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.fgl;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdfs.server.namenode.FSNamesystemLock;
+import org.apache.hadoop.metrics2.lib.MutableRatesWithAggregation;
+
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
+
+/**
+ * Splitting the global FSN lock into FSLock and BMLock.
+ * FSLock is used to protect directory tree-related operations.
+ * BMLock is used to protect block-related and dn-related operations.
+ * The lock order should be: FSLock,BMLock.
+ */
+public class FineGrainedFSNamesystemLock implements FSNLockManager {
+  private final FSNamesystemLock fsLock;
+  private final FSNamesystemLock bmLock;
+
+  public FineGrainedFSNamesystemLock(Configuration conf, 
MutableRatesWithAggregation aggregation) {
+this.fsLock = new FSNamesystemLock(conf, "FS", aggregation);
+this.bmLock = new FSNamesystemLock(conf, "BM", aggregation);
+  }
+
+  @Override
+  public void readLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLock();
+  this.bmLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLock();
+}
+  }
+
+  public void readLockInterruptibly(FSNamesystemLockMode lockMode) throws 
InterruptedException  {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.readLockInterruptibly();
+  try {
+this.bmLock.readLockInterruptibly();
+  } catch (InterruptedException e) {
+// The held FSLock should be released if the current thread is 
interrupted
+// while acquiring the BMLock.
+this.fsLock.readUnlock("BMReadLockInterruptiblyFailed");
+throw e;
+  }
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readLockInterruptibly();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readLockInterruptibly();
+}
+  }
+
+  @Override
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName);
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName);
+}
+  }
+
+  public void readUnlock(FSNamesystemLockMode lockMode, String opName,
+  Supplier lockReportInfoSupplier) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.readUnlock(opName, lockReportInfoSupplier);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.readUnlock(opName, lockReportInfoSupplier);
+}
+  }
+
+  @Override
+  public void writeLock(FSNamesystemLockMode lockMode) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.fsLock.writeLock();
+  this.bmLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeLock();
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.writeLock();
+}
+  }
+
+  @Override
+  public void writeUnlock(FSNamesystemLockMode lockMode, String opName) {
+if (lockMode.equals(FSNamesystemLockMode.GLOBAL)) {
+  this.bmLock.writeUnlock(opName);
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.FS)) {
+  this.fsLock.writeUnlock(opName);
+} else if (lockMode.equals(FSNamesystemLockMode.BM)) {
+  this.bmLock.wr

Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-05-12 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2106553639

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | -1 :x: |  patch  |   1m 02s |  |  
https://github.com/apache/hadoop/pull/6762 does not apply to trunk. Rebase 
required? Wrong Branch? See 
https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.  
|
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6762/3/console
 |
   | versions | git=2.44.0.windows.1 |
   | 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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-05-06 Thread via GitHub


ZanderXu commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1591744778


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -6043,7 +6079,7 @@ void updatePipeline(
   updatePipelineInternal(clientName, oldBlock, newBlock, newNodes,
   newStorageIDs, logRetryCache);
 } finally {
-  writeUnlock("updatePipeline");
+  writeUnlock(FSNamesystemLockMode.GLOBAL, "updatePipeline");

Review Comment:
   Yes, this RPC involves iNode and FSEditLog, so we use global lock 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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-05-04 Thread via GitHub


ferhui commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1590192211


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -6043,7 +6079,7 @@ void updatePipeline(
   updatePipelineInternal(clientName, oldBlock, newBlock, newNodes,
   newStorageIDs, logRetryCache);
 } finally {
-  writeUnlock("updatePipeline");
+  writeUnlock(FSNamesystemLockMode.GLOBAL, "updatePipeline");

Review Comment:
   That documentation is the final solution. Currently this PR is the 1st 
stage. This stage only aims to split the global lock into bm and fs locks.
   In updatePipelineInternal we can see it uses inode too, so we use global 
lock here. Will solve similar cases in 2nd stage.
   @ZanderXu  any other comments?



-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-04-28 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2081720138

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m 14s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  spotbugs  |   0m 01s |  |  spotbugs executables are not 
available.  |
   | +0 :ok: |  codespell  |   0m 01s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m 01s |  |  detect-secrets was not available.  
|
   | +0 :ok: |  xmllint  |   0m 01s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m 01s |  |  markdownlint was not available.  
|
   | +1 :green_heart: |  @author  |   0m 00s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m 00s |  |  The patch appears to 
include 47 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 18s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  87m 56s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  39m 10s |  |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   7m 54s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |  15m 54s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |  14m 59s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  | 174m 23s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 14s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   9m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  36m 45s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |  36m 44s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m 01s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   5m 53s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |  16m 04s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |  14m 57s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  | 183m 47s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   6m 03s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 552m 56s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint 
markdownlint |
   | uname | MINGW64_NT-10.0-17763 087b28dd158d 3.4.10-87d57229.x86_64 
2024-02-14 20:17 UTC x86_64 Msys |
   | Build tool | maven |
   | Personality | /c/hadoop/dev-support/bin/hadoop.sh |
   | git revision | trunk / 14153f07aab8a229f128f503b2741d1c81a824b2 |
   | Default Java | Azul Systems, Inc.-1.8.0_332-b09 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6762/2/testReport/
 |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs 
hadoop-hdfs-project/hadoop-hdfs-rbf hadoop-tools/hadoop-fs2img U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6762/2/console
 |
   | versions | git=2.44.0.windows.1 |
   | 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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-04-24 Thread via GitHub


hfutatzhanghb commented on code in PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#discussion_r1578763344


##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -6043,7 +6079,7 @@ void updatePipeline(
   updatePipelineInternal(clientName, oldBlock, newBlock, newNodes,
   newStorageIDs, logRetryCache);
 } finally {
-  writeUnlock("updatePipeline");
+  writeUnlock(FSNamesystemLockMode.GLOBAL, "updatePipeline");

Review Comment:
   @ZanderXu Hi, sir. Have a question here. In documentaion, it is said that 
updateBlockForPipeline and updatePipeline are only involved by blocks 
operations. Why we use FSNamesystemLockMode.GLOBAL 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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-04-23 Thread via GitHub


ZanderXu commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2071967284

   The spotbugs will be fixed by HDFS-17451. And the failed UT is not imported 
by this PR.


-- 
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-unsubscr...@hadoop.apache.org

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


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



Re: [PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-04-23 Thread via GitHub


hadoop-yetus commented on PR #6762:
URL: https://github.com/apache/hadoop/pull/6762#issuecomment-2071959089

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 30s |  |  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.  
|
   | +0 :ok: |  xmllint  |   0m  1s |  |  xmllint was not available.  |
   | +0 :ok: |  markdownlint  |   0m  1s |  |  markdownlint 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 47 new or modified test files.  |
    _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  14m 52s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  32m 14s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  17m 33s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  compile  |  16m 26s |  |  trunk passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06  |
   | +1 :green_heart: |  checkstyle  |   4m 34s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 19s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   3m  0s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 26s |  |  trunk passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06  |
   | -1 :x: |  spotbugs  |   1m 34s | 
[/branch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf-warnings.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/1/artifact/out/branch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf-warnings.html)
 |  hadoop-hdfs-project/hadoop-hdfs-rbf in trunk has 1 extant spotbugs 
warnings.  |
   | +1 :green_heart: |  shadedclient  |  34m 18s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 34s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m  3s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 51s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javac  |  16m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 21s |  |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06  |
   | +1 :green_heart: |  javac  |  16m 21s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   4m 36s |  |  root: The patch generated 
0 new + 1232 unchanged - 16 fixed = 1232 total (was 1248)  |
   | +1 :green_heart: |  mvnsite  |   3m 22s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 55s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   3m 32s |  |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06  |
   | +1 :green_heart: |  spotbugs  |   6m 34s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  34m 14s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  | 237m  5s | 
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
 |  hadoop-hdfs in the patch passed.  |
   | +1 :green_heart: |  unit  |  29m 40s |  |  hadoop-hdfs-rbf in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   0m 50s |  |  hadoop-fs2img in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m  9s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 506m  7s |  |  |
   
   
   | Reason | Tests |
   |---:|:--|
   | Failed junit tests | hadoop.hdfs.server.datanode.TestLargeBlockReport |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6762/1/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6762 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint 
markdownlint |
   | uname | Linux ac7b975b9e42 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git r

[PR] HDFS-17384. [FGL] Replace the global lock with global FS Lock and global BM lock [hadoop]

2024-04-22 Thread via GitHub


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

   We plan to merge HDFS-17384 to the trunk branch. 
   
   This PR is used to review all changes in HDFS-17384.


-- 
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-unsubscr...@hadoop.apache.org

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


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