HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. #5120

sauraank · 2022-11-08T17:45:05Z

Description of PR

Set the lower limit on s3a prefetching/caching block size as 1 byte. Not removed the limit as it would essentially result in infinite number of prefetching/caching request when the block size is 0 byte.

How was this patch tested?

Tested in eu-west-1 by running mvn -Dparallel-tests clean verify. Not written a separate test as the change was trivial. Following is the result -
[INFO] Results:
[INFO]
[WARNING] Tests run: 1154, Failures: 0, Errors: 0, Skipped: 182
[INFO]
[INFO]
[INFO] --- maven-failsafe-plugin:3.0.0-M1:integration-test (sequential-integration-tests) @ hadoop-aws ---
[INFO]

[INFO] Results:
[INFO]
[WARNING] Tests run: 124, Failures: 0, Errors: 0, Skipped: 84
[INFO]
[INFO]
[INFO] --- maven-enforcer-plugin:3.0.0:enforce (depcheck) @ hadoop-aws ---
[INFO]
[INFO] --- maven-failsafe-plugin:3.0.0-M1:verify (default-integration-test) @ hadoop-aws ---
[INFO]
[INFO] --- maven-failsafe-plugin:3.0.0-M1:verify (sequential-integration-tests) @ hadoop-aws ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 13:52 min
[INFO] Finished at: 2022-11-10T15:35:44Z
[INFO] Final Memory: 68M/1402M
[INFO] ------------------------------------------------------------------------

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?

ahmarsuhail

looks good, some suggested changes for the readme. Can you also update the documentation in https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md#general-s3a-client-configuration for the block size property.

ahmarsuhail · 2022-11-09T04:02:51Z

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/prefetching.md

+Although, default size of the block for prefetching the input stream is 8 MB, 
+minimum size allowed to set is 1 byte for a block.
+User should set the block size with the understanding that smaller block sizes increases the number of blocks.
+Thus, smaller block size affects the performance by increasing the overhead for reading and prefetching
+each block.


Suggested change

Although, default size of the block for prefetching the input stream is 8 MB,

minimum size allowed to set is 1 byte for a block.

User should set the block size with the understanding that smaller block sizes increases the number of blocks.

Thus, smaller block size affects the performance by increasing the overhead for reading and prefetching

each block.

The default size of a block is 8MB, and the minimum allowed block size is 1 byte.

Decreasing block size will increase the number of blocks to be read for a file.

A smaller block size may negatively impact performance as the number of prefetches required will increase.

sauraank · 2022-11-09T16:37:08Z

Thanks @ahmarsuhail for the feedback. I have made the recommended changes.

hadoop-yetus · 2022-11-09T18:24:50Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 50s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+0 🆗	markdownlint	0m 1s		markdownlint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	41m 58s		trunk passed
+1 💚	compile	0m 52s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	0m 42s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 40s		trunk passed
+1 💚	mvnsite	0m 49s		trunk passed
+1 💚	javadoc	0m 35s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 37s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 25s		trunk passed
+1 💚	shadedclient	23m 57s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 43s		the patch passed
+1 💚	compile	0m 47s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	0m 47s		the patch passed
+1 💚	compile	0m 33s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 33s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚	checkstyle	0m 25s		the patch passed
+1 💚	mvnsite	0m 39s		the patch passed
+1 💚	javadoc	0m 27s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 44s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 56s		the patch passed
+1 💚	shadedclient	29m 0s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 48s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 40s		The patch does not generate ASF License warnings.
		112m 6s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/2/artifact/out/Dockerfile
GITHUB PR	#5120
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint
uname	Linux edc34f5eff5e 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 / `bf73b01`
Default Java	Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/2/testReport/
Max. process+thread count	533 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/2/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.

ahmarsuhail · 2022-11-10T03:51:10Z

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md

+      Default value is 8 MB.
+      Lower limit for the block size is 1 byte.


Sorry, I should have updated my comment. We discussed offline, you can either revert this change completely, or change to just the following:

Suggested change

Default value is 8 MB.

Lower limit for the block size is 1 byte.

Decreasing this will increase the number of prefetches required, and may negatively impact performance.

ahmarsuhail

looks good, +1. Please update the description of the PR with your test results.

hadoop-yetus · 2022-11-10T13:25:05Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 55s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+0 🆗	markdownlint	0m 1s		markdownlint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	42m 11s		trunk passed
+1 💚	compile	0m 52s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	0m 42s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 40s		trunk passed
+1 💚	mvnsite	0m 49s		trunk passed
+1 💚	javadoc	0m 34s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 38s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 23s		trunk passed
+1 💚	shadedclient	23m 43s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 34s		the patch passed
+1 💚	compile	0m 40s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	0m 40s		the patch passed
+1 💚	compile	0m 32s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 32s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 23s		the patch passed
+1 💚	mvnsite	0m 37s		the patch passed
+1 💚	javadoc	0m 17s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 26s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 12s		the patch passed
+1 💚	shadedclient	23m 27s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 43s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 40s		The patch does not generate ASF License warnings.
		104m 45s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/3/artifact/out/Dockerfile
GITHUB PR	#5120
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint
uname	Linux 1081b01577c1 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 / `5705e99`
Default Java	Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/3/testReport/
Max. process+thread count	531 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5120/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.

dannycjones

+1 (non-binding), lgtm.

Yetus -1 for no new tests is to be expected.

sauraank · 2023-02-02T11:18:51Z

Hey @steveloughran , please review this PR about reducing the lower limit on prefetching/caching block size. Reduced it to 1 byte.
Thanks.

steveloughran

+1. no reason for a test for this that I can see

…yte. (apache#5120) The minimum value of fs.s3a.prefetch.block.size is now 1 Contributed by Ankit Saurabh

…yte. (#5120) The minimum value of fs.s3a.prefetch.block.size is now 1 Contributed by Ankit Saurabh

Ankit Saurabh added 4 commits November 4, 2022 15:24

Made the lower limit to 1

0429180

Lowered the Prefetch Limit, unchanged the Default size

cdbed32

Added the lower limit information in the Prefetch Readme

563f0ff

Added new line after period in readme.

ddba5cd

ahmarsuhail requested changes Nov 9, 2022

View reviewed changes

Updated the documentation of general s3a client configuration.

bf73b01

ahmarsuhail requested changes Nov 10, 2022

View reviewed changes

Improved block description in s3a client configuration

5705e99

ahmarsuhail approved these changes Nov 10, 2022

View reviewed changes

dannycjones approved these changes Nov 10, 2022

View reviewed changes

sauraank changed the title ~~HADOOP-18246. Remove lower limit on s3a prefetching/caching block size~~ HADOOP-18246. prefetch lower limit Nov 10, 2022

sauraank changed the title ~~HADOOP-18246. prefetch lower limit~~ HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. Nov 10, 2022

steveloughran approved these changes Feb 2, 2023

View reviewed changes

steveloughran merged commit 22f6d55 into apache:trunk Feb 2, 2023

ahmarsuhail mentioned this pull request Apr 17, 2023

HADOOP-18028. High performance S3A input stream (#4752) #5559

Closed

4 tasks

ferdelyi pushed a commit to ferdelyi/hadoop that referenced this pull request May 26, 2023

HADOOP-18246. Reduce lower limit on fs.s3a.prefetch.block.size to 1 b…

48ede86

…yte. (apache#5120) The minimum value of fs.s3a.prefetch.block.size is now 1 Contributed by Ankit Saurabh

asfgit pushed a commit that referenced this pull request Jul 25, 2023

HADOOP-18246. Reduce lower limit on fs.s3a.prefetch.block.size to 1 b…

f439801

…yte. (#5120) The minimum value of fs.s3a.prefetch.block.size is now 1 Contributed by Ankit Saurabh

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. #5120

HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. #5120

Uh oh!

sauraank commented Nov 8, 2022 •

edited

Loading

Uh oh!

ahmarsuhail left a comment

Uh oh!

ahmarsuhail Nov 9, 2022

Uh oh!

sauraank commented Nov 9, 2022

Uh oh!

hadoop-yetus commented Nov 9, 2022

Uh oh!

ahmarsuhail Nov 10, 2022

Uh oh!

ahmarsuhail left a comment

Uh oh!

hadoop-yetus commented Nov 10, 2022

Uh oh!

dannycjones left a comment

Uh oh!

sauraank commented Feb 2, 2023

Uh oh!

steveloughran left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		Default value is 8 MB.
		Lower limit for the block size is 1 byte.

	Default value is 8 MB.
	Lower limit for the block size is 1 byte.
	Decreasing this will increase the number of prefetches required, and may negatively impact performance.

HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. #5120

HADOOP-18246. Reduce lower limit on s3a prefetching/caching block size to 1 byte. #5120

Uh oh!

Conversation

sauraank commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

How was this patch tested?

For code changes:

Uh oh!

ahmarsuhail left a comment

Choose a reason for hiding this comment

Uh oh!

ahmarsuhail Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

sauraank commented Nov 9, 2022

Uh oh!

hadoop-yetus commented Nov 9, 2022

Uh oh!

ahmarsuhail Nov 10, 2022

Choose a reason for hiding this comment

Uh oh!

ahmarsuhail left a comment

Choose a reason for hiding this comment

Uh oh!

hadoop-yetus commented Nov 10, 2022

Uh oh!

dannycjones left a comment

Choose a reason for hiding this comment

Uh oh!

sauraank commented Feb 2, 2023

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sauraank commented Nov 8, 2022 •

edited

Loading