HDFS-15025. Applying NVDIMM storage media to HDFS #2189

huangtianhua · 2020-08-04T03:23:29Z

The non-volatile memory NVDIMM is faster than SSD,
it can be used simultaneously with RAM, DISK and SSD.
Storing the data of HDFS on NVDIMM directly will get
better response rate and reliability.

huangtianhua · 2020-08-17T06:34:47Z

@liuml07 Hi, would you please help to review it then we will modify this ASAP, thanks very much:)

liuml07

The overall change looks good to me, thanks! I will finish the review of testing in 1/2 days and provide more input.

I also will check which isTransient() will need to be replaced with isRAM(). It seems case by case for all usages. One simple question is for this NVDIMM storage type, we save the checksum file right?

...dfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockStoragePolicySuite.java

hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/ArchivalStorage.md

brahmareddybattula

At first glance I have couple of comments/queries

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/StorageType.java

.../hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsVolumeSpi.java

hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/ArchivalStorage.md

huangtianhua · 2020-09-02T07:20:48Z

@liuml07 and @brahmareddybattula please help to review the latest code again? thanks very much.

huangtianhua · 2020-09-11T02:07:55Z

@liuml07 and @brahmareddybattula So if all of the codes are ok, would you please to approve? Thanks.

liuml07 · 2020-09-11T02:43:06Z

Will check again later this week.

Ideally we can get a clean QA. Could you check the test failures and make sure they are not related? Thanks,

huangtianhua · 2020-09-11T06:53:48Z

@liuml07 We have checked for several times, the failure tests are not related with this patch, and I found almost all of the HDFS-* patches are not pass the hadoop-hdfs unit tests, like #2275 #2205
So maybe the failure tests are not blocker?

liuml07 · 2020-09-11T07:42:15Z

No this is not a blocker @huangtianhua I will review (again) and commit hopefully this week if there is no other comments. Thanks,

huangtianhua · 2020-09-14T01:23:57Z

@liuml07 That sounds good, thank you very much:)

huangtianhua · 2020-09-14T01:25:27Z

@brahmareddybattula Hi brahma, would you review it again, thanks.

huangtianhua · 2020-09-17T01:31:46Z

@liuml07 and @brahmareddybattula Sorry to disturb you again, I wonder if there is any update about this patch?

liuml07 · 2020-09-17T01:41:16Z

Sorry, I have been busy in other stuff. The recent review I have no other comments except a few trivial ones. I'll post that and commit this week.

liuml07

+1

Left some minor comments.

Will commit directly after comments are fixed and new QA is clean or failing tests are checked.

...-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java

...dfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/net/TestDFSNetworkTopology.java

...p-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockStatsMXBean.java

...s-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataDirs.java

...fs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsVolumeList.java

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java

Need more change in code

huangtianhua · 2020-09-23T01:29:39Z

@liuml07 Sorry, we didn't catch you, what other changes we have to do?

liuml07 · 2020-09-23T02:05:55Z

No more comments on the latest commit.

Let's wait for the QA and run failing tests locally. I'll commit shortly.

hadoop-yetus · 2020-09-23T06:32:18Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 35s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	buf	0m 1s		buf was not available.
+0 🆗	markdownlint	0m 1s		markdownlint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 16 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	3m 24s		Maven dependency ordering for branch
+1 💚	mvninstall	32m 19s		trunk passed
+1 💚	compile	26m 40s		trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	compile	22m 27s		trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	checkstyle	3m 41s		trunk passed
+1 💚	mvnsite	4m 32s		trunk passed
+1 💚	shadedclient	26m 48s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 29s		trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	4m 13s		trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+0 🆗	spotbugs	3m 16s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	10m 3s		trunk passed
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 27s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 37s		the patch passed
+1 💚	compile	20m 54s		the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
-1 ❌	cc	20m 54s	/diff-compile-cc-root-jdkUbuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1.txt	root-jdkUbuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 generated 32 new + 131 unchanged - 32 fixed = 163 total (was 163)
+1 💚	javac	20m 54s		the patch passed
+1 💚	compile	16m 48s		the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
-1 ❌	cc	16m 48s	/diff-compile-cc-root-jdkPrivateBuild-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01.txt	root-jdkPrivateBuild-1.8.0_265-8u265-b01-0ubuntu2~~18.04-b01 with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~~18.04-b01 generated 37 new + 126 unchanged - 37 fixed = 163 total (was 163)
+1 💚	javac	16m 48s		the patch passed
-0 ⚠️	checkstyle	2m 57s	/diff-checkstyle-root.txt	root: The patch generated 16 new + 723 unchanged - 6 fixed = 739 total (was 729)
+1 💚	mvnsite	4m 3s		the patch passed
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	xml	0m 1s		The patch has no ill-formed XML file.
+1 💚	shadedclient	14m 15s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 34s		the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1
+1 💚	javadoc	4m 6s		the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
+1 💚	findbugs	8m 21s		the patch passed
			_ Other Tests _
+1 💚	unit	9m 22s		hadoop-common in the patch passed.
+1 💚	unit	2m 21s		hadoop-hdfs-client in the patch passed.
-1 ❌	unit	96m 8s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 9s		The patch does not generate ASF License warnings.
		322m 9s

Reason	Tests
Failed junit tests	hadoop.hdfs.TestSnapshotCommands
	hadoop.hdfs.TestGetFileChecksum
	hadoop.hdfs.TestFileChecksumCompositeCrc
	hadoop.hdfs.server.namenode.TestNameNodeRetryCacheMetrics
	hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
	hadoop.hdfs.server.datanode.TestBPOfferService
	hadoop.hdfs.server.sps.TestExternalStoragePolicySatisfier
	hadoop.hdfs.TestFileChecksum

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2189/17/artifact/out/Dockerfile
GITHUB PR	#2189
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle cc buflint bufcompat xml markdownlint
uname	Linux b39fec604d95 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `ae089f2`
Default Java	Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2189/17/testReport/
Max. process+thread count	4531 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2189/17/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

liuml07

+1

cc failure is not related in hadoop-mapreduce-client module for both Java 8 and Java 11
checkstyle error is related. Please fix that.
unit tests are not related and should be addressed separately

The non-volatile memory NVDIMM is faster than SSD, it can be used simultaneously with RAM, DISK and SSD. Storing the data of HDFS on NVDIMM directly will get better response rate and reliability. Co-authored-by: YaYun-Wang <yywangyayun@163.com>

put NVDIMM to the end of all storage types

add a blank line before every new rack

add one space after 'disk'

add NVDIMM test for setQuotaByStorageType method

update assertEquals() with two parameters

huangtianhua · 2020-09-24T02:16:59Z

@liuml07 , thanks, and we have updated the code, seems there is a checkstyle about the storage enum name of 'nvdimm' should keep as it is to consistent with other storage types, what do you think?

liuml07 · 2020-09-24T02:41:05Z

Yeah; for this checkstyle in test we can keep it as-is for the sake of code consistency. Thanks.

brahmareddybattula

+1

brahmareddybattula · 2020-09-24T08:55:03Z

@huangtianhua and @YaYun-Wang thanks for contribution. @liuml07 thanks for review.

* HDFS-15025. Applying NVDIMM storage media to HDFS Co-authored-by: YaYun-Wang <yywangyayun@163.com> Co-authored-by: YaYun-Wang <34060507+YaYun-Wang@users.noreply.github.com>

huangtianhua force-pushed the HDFS-15025 branch 3 times, most recently from a1caf08 to b8286dc Compare August 4, 2020 09:40

liuml07 assigned huangtianhua Aug 4, 2020

liuml07 requested review from liuml07 and jojochuang August 4, 2020 16:28

liuml07 added the HDFS label Aug 4, 2020

huangtianhua force-pushed the HDFS-15025 branch from b8286dc to bb02bd8 Compare August 20, 2020 03:41

liuml07 reviewed Aug 24, 2020

View reviewed changes

...dfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockStoragePolicySuite.java Outdated Show resolved Hide resolved

hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/ArchivalStorage.md Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

huangtianhua force-pushed the HDFS-15025 branch from 9e1f06d to 84c3078 Compare August 25, 2020 08:22

This comment has been minimized.

Sign in to view

brahmareddybattula reviewed Aug 25, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

liuml07 previously approved these changes Sep 20, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

apache deleted a comment from hadoop-yetus Sep 23, 2020

liuml07 approved these changes Sep 23, 2020

View reviewed changes

huangtianhua and others added 12 commits September 24, 2020 09:23

Update ArchivalStorage.md

17fd047

Update ArchivalStorage.md

76d2716

put NVDIMM to the end of all storage types

Update FsDatasetImpl.java

075ee8a

Update TestDFSNetworkTopology.java

180cc03

add a blank line before every new rack

Update TestDFSNetworkTopology.java

0d753ad

Update TestDFSNetworkTopology.java

8f9ace0

Update TestBlockStatsMXBean.java

842f8c7

Update TestDataDirs.java

8b75c79

add one space after 'disk'

Update TestWebHDFS.java

241faf4

add NVDIMM test for setQuotaByStorageType method

Update TestFsVolumeList.java

7771df7

update assertEquals() with two parameters

Fix checkstyle errors

0a87e83

huangtianhua force-pushed the HDFS-15025 branch from 82464cb to 0a87e83 Compare September 24, 2020 02:02

brahmareddybattula self-requested a review September 24, 2020 04:52

brahmareddybattula approved these changes Sep 24, 2020

View reviewed changes

brahmareddybattula merged commit ff59fbb into apache:trunk Sep 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDFS-15025. Applying NVDIMM storage media to HDFS #2189

HDFS-15025. Applying NVDIMM storage media to HDFS #2189

huangtianhua commented Aug 4, 2020

huangtianhua commented Aug 17, 2020

liuml07 left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

brahmareddybattula left a comment

This comment has been minimized.

This comment has been minimized.

huangtianhua commented Sep 2, 2020

huangtianhua commented Sep 11, 2020

liuml07 commented Sep 11, 2020

huangtianhua commented Sep 11, 2020

liuml07 commented Sep 11, 2020

huangtianhua commented Sep 14, 2020

huangtianhua commented Sep 14, 2020

huangtianhua commented Sep 17, 2020

liuml07 commented Sep 17, 2020

liuml07 left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

huangtianhua commented Sep 23, 2020

liuml07 commented Sep 23, 2020

hadoop-yetus commented Sep 23, 2020

liuml07 left a comment

huangtianhua commented Sep 24, 2020

liuml07 commented Sep 24, 2020

brahmareddybattula left a comment

brahmareddybattula commented Sep 24, 2020 •

edited

Loading

HDFS-15025. Applying NVDIMM storage media to HDFS #2189

HDFS-15025. Applying NVDIMM storage media to HDFS #2189

Conversation

huangtianhua commented Aug 4, 2020

huangtianhua commented Aug 17, 2020

liuml07 left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

brahmareddybattula left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

huangtianhua commented Sep 2, 2020

huangtianhua commented Sep 11, 2020

liuml07 commented Sep 11, 2020

huangtianhua commented Sep 11, 2020

liuml07 commented Sep 11, 2020

huangtianhua commented Sep 14, 2020

huangtianhua commented Sep 14, 2020

huangtianhua commented Sep 17, 2020

liuml07 commented Sep 17, 2020

liuml07 left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

huangtianhua commented Sep 23, 2020

liuml07 commented Sep 23, 2020

hadoop-yetus commented Sep 23, 2020

liuml07 left a comment

Choose a reason for hiding this comment

huangtianhua commented Sep 24, 2020

liuml07 commented Sep 24, 2020

brahmareddybattula left a comment

Choose a reason for hiding this comment

brahmareddybattula commented Sep 24, 2020 • edited Loading

brahmareddybattula commented Sep 24, 2020 •

edited

Loading