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 2021/03/30 11:21:37 UTC
[GitHub] [hadoop] sodonnel opened a new pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
sodonnel opened a new pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838
When the datanode block layout is upgrade from -56 (256x256) to -57 (32x32), we have found the datanode uses a lot more memory than usual.
For each volume, the blocks are scanned and a list is created holding a series of LinkArgs objects. This object contains a File object for the block source and destination. The file object stores the path as a string, eg:
/data01/dfs/dn/current/BP-586623041-127.0.0.1-1617017575175/current/finalized/subdir0/subdir0/blk_1073741825_1001.meta
/data01/dfs/dn/current/BP-586623041-127.0.0.1-1617017575175/current/finalized/subdir0/subdir0/blk_1073741825
This is string is repeated for every block and meta file on the DN, and much of the string is the same each time, leading to a large amount of memory.
If we change the linkArgs to store:
* Src Path without the block, eg /data01/dfs/dn/previous.tmp/BP-586623041-127.0.0.1-1617017575175/current/finalized/subdir0/subdir0
* Dest Path without the block eg /data01/dfs/dn/current/BP-586623041-127.0.0.1-1617017575175/current/finalized/subdir0/subdir10
* Block / Meta file name, eg blk_12345678_1001 or blk_12345678_1001.meta
Then ensure were reuse the same file object for repeated src and dest paths, we can save most of the memory without reworking the logic of the code.
The current logic works along the source paths recursively, so you can easily re-use the src path object.
For the destination path, there are only 32x32 (1024) distinct paths, so we can simply cache them in a hashMap and lookup the re-useable object each time.
I tested locally by generating 100k block files and attempting the layout upgrade. A heap dump showed the 100k blocks using about 140MB of heap. That is close to 1.5GB per 1M blocks.
After the change outlined above the same 100K blocks used about 20MB of heap, so 200MB per million blocks.
A general DN sizing recommendation is 1GB of heap per 1M blocks, so the upgrade should be able to happen within the pre-upgrade heap.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#issuecomment-810412030
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 38s | | trunk passed |
| +1 :green_heart: | compile | 1m 25s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 1m 12s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | checkstyle | 0m 59s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 21s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 24s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 4s | | trunk passed |
| +1 :green_heart: | shadedclient | 15m 59s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 9s | | the patch passed |
| +1 :green_heart: | compile | 1m 13s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 1m 13s | | the patch passed |
| +1 :green_heart: | compile | 1m 10s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | javac | 1m 10s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 55s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/1/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 31 unchanged - 2 fixed = 32 total (was 33) |
| +1 :green_heart: | mvnsite | 1m 21s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 49s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 19s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 5s | | the patch passed |
| +1 :green_heart: | shadedclient | 16m 0s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 232m 30s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 318m 50s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| | hadoop.hdfs.server.datanode.TestBlockRecovery |
| | hadoop.hdfs.server.namenode.snapshot.TestNestedSnapshots |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2838 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 70d639a759ad 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 57b6db1f552b356c87978fc7a04ccc8e5927d690 |
| Default Java | Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/1/testReport/ |
| Max. process+thread count | 3420 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT 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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel commented on pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
sodonnel commented on pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#issuecomment-811876392
Thanks @Hexiaoqiao ! @jojochuang have you got any more comments or do you want to take another look before I commit 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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jojochuang commented on a change in pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#discussion_r604604531
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1071,12 +1071,26 @@ private static void linkAllBlocks(File fromDir, File fromBbwDir, File toDir,
}
private static class LinkArgs {
- File src;
- File dst;
+ private File srcDir;
+ private File dstDir;
+ private String blockFile;
+
+ LinkArgs(File srcDir, File dstDir, String blockFile) {
+ this.srcDir = srcDir;
+ this.dstDir = dstDir;
+ this.blockFile = blockFile;
+ }
+
+ public File src() {
Review comment:
it was meant to cause less code churn --> src ==> src() :)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#issuecomment-810647490
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 49s | | trunk passed |
| +1 :green_heart: | compile | 1m 20s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 1m 13s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 3s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 22s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 56s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 28s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 8s | | trunk passed |
| +1 :green_heart: | shadedclient | 16m 2s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 7s | | the patch passed |
| +1 :green_heart: | compile | 1m 10s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 1m 10s | | the patch passed |
| +1 :green_heart: | compile | 1m 8s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | javac | 1m 8s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 52s | | hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 30 unchanged - 3 fixed = 30 total (was 33) |
| +1 :green_heart: | mvnsite | 1m 13s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 45s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 16s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 15m 45s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 227m 53s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 45s | | The patch does not generate ASF License warnings. |
| | | 312m 54s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| | hadoop.hdfs.TestRollingUpgrade |
| | hadoop.hdfs.server.datanode.TestBlockRecovery |
| | hadoop.hdfs.server.namenode.snapshot.TestNestedSnapshots |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2838 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 0359a8b54474 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 6c30ed2108877a3e628a4f52b4e4578f769da7f7 |
| Default Java | Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/2/testReport/ |
| Max. process+thread count | 2963 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT 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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel merged pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
sodonnel merged pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel commented on a change in pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#discussion_r604757437
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1071,12 +1071,26 @@ private static void linkAllBlocks(File fromDir, File fromBbwDir, File toDir,
}
private static class LinkArgs {
- File src;
- File dst;
+ private File srcDir;
+ private File dstDir;
+ private String blockFile;
+
+ LinkArgs(File srcDir, File dstDir, String blockFile) {
+ this.srcDir = srcDir;
+ this.dstDir = dstDir;
+ this.blockFile = blockFile;
+ }
+
+ public File src() {
Review comment:
Yea I kept src() and dst() just to reduce the changes in the rest of the code and make it easier to review. Then I used blockFile rather than getBlockFile to keep it consistent.
I think its OK like this, as this is an inner class only used internally by DataStorage.
What do you think?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel commented on a change in pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#discussion_r604770420
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1345,8 +1363,18 @@ public boolean accept(File dir, String name) {
throw new IOException("Failed to mkdirs " + blockLocation);
}
}
- idBasedLayoutSingleLinks.add(new LinkArgs(new File(from, blockName),
- new File(blockLocation, blockName)));
+ /**
+ * The destination path is 32x32, so 1024 distinct paths. Therefore
+ * we cache the destination path and reuse the same File object on
+ * potentially thousands of blocks located on this volume.
+ * This method is called recursively so the cache is passed through
+ * each recursive call. There is one cache per volume, and it is only
+ * accessed by a single thread so no locking is needed.
+ */
+ File cachedDest = pathCache
+ .computeIfAbsent(blockLocation, k -> blockLocation);
+ idBasedLayoutSingleLinks.add(new LinkArgs(from,
Review comment:
Yes. This is the part of the change that really reduces the memory. As src blocks are checked, it will produce destination paths like:
```
/dest/subdir1/subdir10 ***
/dest/subdir12/subdir1
/dest/subdir3/subdir1
/dest/subdir1/subdir10 *** These two are the same, but unless we cache the instance we cannot reuse the object
```
There are only 1024 unique paths, but if we just use the calculated path each time, we will have thousands of File objects each with the same path stored in them.
We still create a temporary File object with the destination, but then find the unique instance for it in the hashmap and store it in the LinkArg. Then the temporary one will be GC'ed and we will end up with at most 1024 unique file objects for dst, saving a lot of memory.
The same holds for the src path, but due to the way the method works, we don't need to cache them. The method finds all the blocks in the src path during the same call, so we just store the passed in src patch each time.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel commented on a change in pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#discussion_r604765744
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1161,10 +1176,12 @@ public Void call() throws IOException {
*/
@Override
public int compare(LinkArgs a, LinkArgs b) {
+ File asrc = a.src();
+ File bsrc = b.src();
return ComparisonChain.start().
- compare(a.src.getName(), b.src.getName()).
- compare(a.src, b.src).
- compare(a.dst, b.dst).
+ compare(asrc.getName(), bsrc.getName()).
+ compare(asrc, bsrc).
Review comment:
This method already had the two compares, but your comment caused me to look at it more closely.
What this seems to be doing is sorting by the block_filename, then by the path, then by the dst.
Eg, if we had this as src:
/a/blk1
/b/blk2
/c/blk1
The natural sort order by path is as above. However the method would return:
/a/blk1
/c/blk1
/b/blk2
This puts duplicate blocks beside each other in the list, even though they are in different paths. Later in the method it uses that to check for duplicate blocks and then handles them.
Therefore I think we need the chained compares like this. However rather than `a.src.getName()` I could use `a.blockFile()` which should be slightly more efficient.
I will push another commit with that change.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#issuecomment-811158199
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 32s | | trunk passed |
| +1 :green_heart: | compile | 1m 19s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 1m 14s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | checkstyle | 1m 2s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 21s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 26s | | trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 2s | | trunk passed |
| +1 :green_heart: | shadedclient | 16m 0s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 12s | | the patch passed |
| +1 :green_heart: | compile | 1m 14s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 1m 14s | | the patch passed |
| +1 :green_heart: | compile | 1m 3s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | javac | 1m 3s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 54s | | hadoop-hdfs-project/hadoop-hdfs: The patch generated 0 new + 30 unchanged - 3 fixed = 30 total (was 33) |
| +1 :green_heart: | mvnsite | 1m 14s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 46s | | the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 21s | | the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| +1 :green_heart: | spotbugs | 3m 5s | | the patch passed |
| +1 :green_heart: | shadedclient | 15m 49s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 230m 1s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | | The patch does not generate ASF License warnings. |
| | | 315m 2s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestBlockScanner |
| | hadoop.hdfs.TestDFSInotifyEventInputStreamKerberized |
| | hadoop.hdfs.server.datanode.TestBlockRecovery |
| | hadoop.hdfs.server.balancer.TestBalancer |
| | hadoop.hdfs.server.namenode.snapshot.TestNestedSnapshots |
| | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2838 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux e7d77a5eb290 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 559216c798b02b2b2803be64e978d263368e55ef |
| Default Java | Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/3/testReport/ |
| Max. process+thread count | 3222 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2838/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT 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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] Hexiaoqiao commented on a change in pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on a change in pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#discussion_r604588150
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1071,12 +1071,26 @@ private static void linkAllBlocks(File fromDir, File fromBbwDir, File toDir,
}
private static class LinkArgs {
- File src;
- File dst;
+ private File srcDir;
+ private File dstDir;
+ private String blockFile;
+
+ LinkArgs(File srcDir, File dstDir, String blockFile) {
+ this.srcDir = srcDir;
+ this.dstDir = dstDir;
+ this.blockFile = blockFile;
+ }
+
+ public File src() {
Review comment:
`getSrc` shoud be more graceful? The same as method `dst(...)` + `blockFile()`.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1161,10 +1176,12 @@ public Void call() throws IOException {
*/
@Override
public int compare(LinkArgs a, LinkArgs b) {
+ File asrc = a.src();
+ File bsrc = b.src();
return ComparisonChain.start().
- compare(a.src.getName(), b.src.getName()).
- compare(a.src, b.src).
- compare(a.dst, b.dst).
+ compare(asrc.getName(), bsrc.getName()).
+ compare(asrc, bsrc).
Review comment:
is it duplicated comparator between `compare(asrc.getName(), bsrc.getName())` and `compare(asrc, bsrc)`?
For UnixFileSystem, the implement is as following. Not sure if it is same for other FileSystem instance.
> public int compare(File f1, File f2) {
> return f1.getPath().compareTo(f2.getPath());
> }
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
##########
@@ -1345,8 +1363,18 @@ public boolean accept(File dir, String name) {
throw new IOException("Failed to mkdirs " + blockLocation);
}
}
- idBasedLayoutSingleLinks.add(new LinkArgs(new File(from, blockName),
- new File(blockLocation, blockName)));
+ /**
+ * The destination path is 32x32, so 1024 distinct paths. Therefore
+ * we cache the destination path and reuse the same File object on
+ * potentially thousands of blocks located on this volume.
+ * This method is called recursively so the cache is passed through
+ * each recursive call. There is one cache per volume, and it is only
+ * accessed by a single thread so no locking is needed.
+ */
+ File cachedDest = pathCache
+ .computeIfAbsent(blockLocation, k -> blockLocation);
+ idBasedLayoutSingleLinks.add(new LinkArgs(from,
Review comment:
Here it reuses prefix path of blocks and no `File` instances created to reduce memory used, 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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] Hexiaoqiao commented on pull request #2838: HDFS-15937. Reduce memory used during datanode layout upgrade
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on pull request #2838:
URL: https://github.com/apache/hadoop/pull/2838#issuecomment-811026690
Thanks @sodonnel and @jojochuang for your detailed comments. It makes sense to me. +1 from my side.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org