HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for direct buffers #4787

mukund-thakur · 2022-08-22T20:44:57Z

Description of PR

VectoredReadUtils.readInDirectBuffer should allocate a max buffer size, .e.g 4mb, then do repeated reads and copies; this ensures that you don't OOM with many threads doing ranged requests.

How was this patch tested?

Ran the existing tests. Also added a UT for zero byte file ranges.

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

…rect buffers part of HADOOP-18103.

steveloughran

this will scale better. regarding the issue about handling 0 byte ranges, the key thing is "no need to allocate a big buffer". we get that wrong with our block output stream, where we could postpone memory allocation until the first byte is written.

steveloughran · 2022-08-23T09:44:27Z

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

+                                         ByteBuffer buffer) throws IOException {
+    int readBytes = 0;
+    int offset = 0;
+    byte[] tmp = new byte[TMP_BUFFER_MAX_SIZE];


make a min of length, TMP_BUFFER_MAX_SIZE for more efficiency on small reads

that is why I haven't added comments in the code yet

steveloughran

looks good; one minor change on the new function interface, plus the yetus comments...

...oject/hadoop-common/src/main/java/org/apache/hadoop/util/functional/Function4RaisingIOE.java

hadoop-yetus · 2022-08-29T23:20:13Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 59s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	15m 8s		Maven dependency ordering for branch
+1 💚	mvninstall	25m 27s		trunk passed
+1 💚	compile	23m 17s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	20m 29s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 4s		trunk passed
+1 💚	mvnsite	3m 8s		trunk passed
+1 💚	javadoc	2m 28s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 19s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 28s		trunk passed
+1 💚	shadedclient	21m 56s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 30s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 46s		the patch passed
+1 💚	compile	22m 30s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	22m 30s		the patch passed
+1 💚	compile	20m 36s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	20m 36s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	3m 58s	/results-checkstyle-root.txt	root: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	mvnsite	3m 7s		the patch passed
+1 💚	javadoc	2m 34s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 24s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 37s		the patch passed
+1 💚	shadedclient	21m 48s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 29s		hadoop-common in the patch passed.
+1 💚	unit	3m 25s		hadoop-aws in the patch passed.
+1 💚	asflicense	1m 30s		The patch does not generate ASF License warnings.
		235m 21s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/3/artifact/out/Dockerfile
GITHUB PR	#4787
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 01d97e9bb986 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b93601c`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/3/testReport/
Max. process+thread count	3159 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2022-08-30T04:38:28Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 0s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	14m 58s		Maven dependency ordering for branch
+1 💚	mvninstall	25m 30s		trunk passed
+1 💚	compile	23m 1s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	20m 26s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 1s		trunk passed
+1 💚	mvnsite	3m 11s		trunk passed
+1 💚	javadoc	2m 22s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 29s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 33s		trunk passed
+1 💚	shadedclient	22m 5s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 32s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 46s		the patch passed
+1 💚	compile	22m 28s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	22m 28s		the patch passed
+1 💚	compile	20m 32s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	20m 32s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 3s		the patch passed
+1 💚	mvnsite	3m 7s		the patch passed
+1 💚	javadoc	2m 12s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 14s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 38s		the patch passed
+1 💚	shadedclient	21m 46s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 39s		hadoop-common in the patch passed.
+1 💚	unit	3m 20s		hadoop-aws in the patch passed.
+1 💚	asflicense	1m 17s		The patch does not generate ASF License warnings.
		234m 24s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/4/artifact/out/Dockerfile
GITHUB PR	#4787
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 5fe5e658fc69 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `5f76a3f`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/4/testReport/
Max. process+thread count	1662 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4787/4/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

steveloughran

+1

…rect buffers (#4787) part of HADOOP-18103. Contributed By: Mukund Thakur

…rect buffers (apache#4787) part of HADOOP-18103. Contributed By: Mukund Thakur

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for di…

a938b4d

…rect buffers part of HADOOP-18103.

mukund-thakur requested a review from steveloughran August 22, 2022 20:45

steveloughran requested changes Aug 23, 2022

View reviewed changes

mukund-thakur added 2 commits August 24, 2022 17:51

Steve's review comments

7346306

Complex refactoring not sure if this is worth it,

e9c0719

that is why I haven't added comments in the code yet

mukund-thakur requested a review from steveloughran August 26, 2022 19:49

steveloughran requested changes Aug 29, 2022

View reviewed changes

...oject/hadoop-common/src/main/java/org/apache/hadoop/util/functional/Function4RaisingIOE.java Outdated Show resolved Hide resolved

Yetus and review comments

b93601c

Checkstyle

5f76a3f

mukund-thakur requested a review from steveloughran August 30, 2022 17:04

steveloughran approved these changes Aug 31, 2022

View reviewed changes

apache deleted a comment from hadoop-yetus Aug 31, 2022

mukund-thakur merged commit 19830c9 into apache:trunk Aug 31, 2022

mukund-thakur added a commit that referenced this pull request Aug 31, 2022

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for di…

6cc5c92

…rect buffers (#4787) part of HADOOP-18103. Contributed By: Mukund Thakur

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for di…

c75fb98

…rect buffers (apache#4787) part of HADOOP-18103. Contributed By: Mukund Thakur

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for direct buffers #4787

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for direct buffers #4787

Uh oh!

mukund-thakur commented Aug 22, 2022

Uh oh!

steveloughran left a comment

Uh oh!

steveloughran Aug 23, 2022

Uh oh!

steveloughran left a comment

Uh oh!

Uh oh!

hadoop-yetus commented Aug 29, 2022

Uh oh!

hadoop-yetus commented Aug 30, 2022

Uh oh!

steveloughran left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for direct buffers #4787

HADOOP-18391. Improvements in VectoredReadUtils#readVectored() for direct buffers #4787

Uh oh!

Conversation

mukund-thakur commented Aug 22, 2022

Description of PR

How was this patch tested?

For code changes:

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

steveloughran Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hadoop-yetus commented Aug 29, 2022

Uh oh!

hadoop-yetus commented Aug 30, 2022

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants