HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads directory #5818

hdaikoku · 2023-07-07T15:38:21Z

Description of PR

This PR fixes a bug in StagingCommitter, which leaks the staging uploads directory, by deleting the directory in StagingCommitter#cleanup().

How was this patch tested?

Ran the integration tests in org.apache.hadoop.fs.s3a.commit.staging.integration against a LocalStack instance running on my laptop.

    <property>
        <name>fs.s3a.endpoint</name>
        <value>s3.ap-south-1.localhost.localstack.cloud:4566</value>
    </property>

    <property>
        <name>fs.s3a.connection.ssl.enabled</name>
        <value>false</value>
    </property>

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.hadoop.fs.s3a.commit.staging.integration.ITestStagingCommitProtocol
[INFO] Tests run: 24, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 33.196 s - in org.apache.hadoop.fs.s3a.commit.staging.integration.ITestStagingCommitProtocol
[INFO] Running org.apache.hadoop.fs.s3a.commit.staging.integration.ITestDirectoryCommitProtocol
[INFO] Tests run: 25, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 30.62 s - in org.apache.hadoop.fs.s3a.commit.staging.integration.ITestDirectoryCommitProtocol
[INFO] Running org.apache.hadoop.fs.s3a.commit.staging.integration.ITestPartitionedCommitProtocol
[WARNING] Tests run: 24, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 26.316 s - in org.apache.hadoop.fs.s3a.commit.staging.integration.ITestPartitionedCommitProtocol
[INFO] Running org.apache.hadoop.fs.s3a.commit.staging.integration.ITestStagingCommitProtocolFailure
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.166 s - in org.apache.hadoop.fs.s3a.commit.staging.integration.ITestStagingCommitProtocolFailure
[INFO] 
[INFO] Results:
[INFO] 
[WARNING] Tests run: 74, Failures: 0, Errors: 0, Skipped: 1

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?

…directory

hdaikoku · 2023-07-07T15:43:27Z

...tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/staging/StagingCommitter.java

-    Path attemptPath = getJobAttemptPath(context);
-    ignoreIOExceptions(LOG,
-        "Deleting Job attempt Path", attemptPath.toString(),
-        () -> deleteWithWarning(
-            getJobAttemptFileSystem(context),
-            attemptPath,
-            true));


Removed these as attemptPath here is a sub dir under ${UUID}/staging-uploads, which has already been deleted in deleteStagingUploadsParentDirectory().

hadoop-yetus · 2023-07-07T17:50:04Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 37s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 1s		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 2 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	46m 21s		trunk passed
+1 💚	compile	0m 41s		trunk passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1
+1 💚	compile	0m 39s		trunk passed with JDK Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
+1 💚	checkstyle	0m 37s		trunk passed
+1 💚	mvnsite	0m 45s		trunk passed
+1 💚	javadoc	0m 33s		trunk passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 38s		trunk passed with JDK Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
+1 💚	spotbugs	1m 13s		trunk passed
+1 💚	shadedclient	33m 39s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	0m 32s		the patch passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1
+1 💚	javac	0m 32s		the patch passed
+1 💚	compile	0m 28s		the patch passed with JDK Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
+1 💚	javac	0m 28s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 22s		the patch passed
+1 💚	mvnsite	0m 31s		the patch passed
+1 💚	javadoc	0m 18s		the patch passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 27s		the patch passed with JDK Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
+1 💚	spotbugs	1m 6s		the patch passed
+1 💚	shadedclient	33m 18s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 39s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 42s		The patch does not generate ASF License warnings.
		130m 15s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5818/1/artifact/out/Dockerfile
GITHUB PR	#5818
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux aad5e097edc1 4.15.0-212-generic #223-Ubuntu SMP Tue May 23 13:09:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `47aefe5`
Default Java	Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u372-ga~~us1-0ubuntu1~~20.04-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5818/1/testReport/
Max. process+thread count	699 (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-5818/1/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

perfect! nice tests in particular.

steveloughran · 2023-07-08T11:49:02Z

...tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/AbstractITCommitProtocol.java

+      return job;
+    }
+
+    public JobContext getJContext() {


use JobContext and TaskContext in getters, even if field names aren't as informative

actually i'm not worrying about this; will merge as is.

…directory (#5818) Contributed by Harunobu Daikoku

hdaikoku · 2023-07-08T16:13:24Z

Thank you for the review 🙏

steveloughran · 2023-07-08T16:36:44Z

oh, thank for a lovely little patch!

…directory (apache#5818) Contributed by Harunobu Daikoku

HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads …

47aefe5

…directory

hdaikoku commented Jul 7, 2023

View reviewed changes

steveloughran approved these changes Jul 8, 2023

View reviewed changes

steveloughran merged commit e3683a9 into apache:trunk Jul 8, 2023

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

HADOOP-18793. S3A StagingCommitter does not clean up staging-uploads …

f02fa66

…directory (#5818) Contributed by Harunobu Daikoku

jiajunmao pushed a commit to jiajunmao/hadoop-MLEC that referenced this pull request Feb 6, 2024

HADOOP-18793. S3A StagingCommitter does not clean up staging-uploads …

f1b44ef

…directory (apache#5818) Contributed by Harunobu Daikoku

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads directory #5818

HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads directory #5818

hdaikoku commented Jul 7, 2023

hdaikoku Jul 7, 2023

hadoop-yetus commented Jul 7, 2023

steveloughran left a comment

steveloughran Jul 8, 2023

steveloughran Jul 8, 2023

hdaikoku commented Jul 8, 2023

steveloughran commented Jul 8, 2023

HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads directory #5818

HADOOP-18793: S3A StagingCommitter does not clean up staging-uploads directory #5818

Conversation

hdaikoku commented Jul 7, 2023

Description of PR

How was this patch tested?

For code changes:

hdaikoku Jul 7, 2023

Choose a reason for hiding this comment

hadoop-yetus commented Jul 7, 2023

steveloughran left a comment

Choose a reason for hiding this comment

steveloughran Jul 8, 2023

Choose a reason for hiding this comment

steveloughran Jul 8, 2023

Choose a reason for hiding this comment

hdaikoku commented Jul 8, 2023

steveloughran commented Jul 8, 2023