Skip to content

Conversation

@steveloughran
Copy link
Contributor

#2636 updated the docs; this is the code-side change to remove the option of disabling magic committer support in the FS

That was a safety check to stop people enabling it on inconsistent/unguarded buckets. We can stop worrying there.

Before anyone asks: what about inconsistent third party stores: every one else's has always been consistent.

tested: s3 london with -Dparallel-tests -DtestsThreadCount=6 -Dmarkers=delete -Dtest=no -Dfs.s3a.directory.marker.audit=true -Dscale

Looks like I skipped the unit tests there...lets see what yetus says

@steveloughran steveloughran changed the title HADOOP-17483. Remove option to enable/disable magic. It is always on HADOOP-17483. Remove option to enable/disable S3A magic committer . It is always on Jan 21, 2021
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 36m 51s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 0m 0s test4tests The patch appears to include 10 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 37s Maven dependency ordering for branch
+1 💚 mvninstall 28m 44s trunk passed
+1 💚 compile 26m 55s trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 compile 21m 9s trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚 checkstyle 3m 8s trunk passed
+1 💚 mvnsite 2m 29s trunk passed
+1 💚 shadedclient 24m 41s branch has no errors when building and testing our client artifacts.
+1 💚 javadoc 1m 28s trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javadoc 2m 9s trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗 spotbugs 1m 14s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 3m 30s trunk passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 24s Maven dependency ordering for patch
+1 💚 mvninstall 1m 37s the patch passed
+1 💚 compile 21m 12s the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javac 21m 12s the patch passed
+1 💚 compile 18m 29s the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚 javac 18m 29s the patch passed
-0 ⚠️ checkstyle 2m 50s /diff-checkstyle-root.txt root: The patch generated 2 new + 51 unchanged - 1 fixed = 53 total (was 52)
+1 💚 mvnsite 2m 13s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 xml 0m 2s The patch has no ill-formed XML file.
+1 💚 shadedclient 17m 16s patch has no errors when building and testing our client artifacts.
+1 💚 javadoc 1m 26s the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javadoc 2m 10s the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
-1 ❌ findbugs 1m 21s /new-findbugs-hadoop-tools_hadoop-aws.html hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
_ Other Tests _
+1 💚 unit 10m 34s hadoop-common in the patch passed.
+1 💚 unit 1m 23s hadoop-aws in the patch passed.
+1 💚 asflicense 0m 48s The patch does not generate ASF License warnings.
250m 13s
Reason Tests
FindBugs module:hadoop-tools/hadoop-aws
Useless control flow in org.apache.hadoop.fs.s3a.commit.CommitUtils.getS3AFileSystem(Path, Configuration, boolean) At CommitUtils.java:Configuration, boolean) At CommitUtils.java:[line 91]
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2637/1/artifact/out/Dockerfile
GITHUB PR #2637
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle
uname Linux 0532a0f86162 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 06fef5e
Default Java Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2637/1/testReport/
Max. process+thread count 2159 (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-2637/1/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.

Copy link
Member

@liuml07 liuml07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests seem fine. +1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecate this method as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@steveloughran
Copy link
Contributor Author

I was thinking about this a bit more and worrying about "maybe I should leave the switch in but true everywhere and only document in minimal terms"

Why so? Because it is doing something unusual: renaming paths. If anyone ever tried to do a distcp from, say, hdfs and there were __magic paths in there, they'd trigger the behaviour. With a switch to turn it off for those operations, at least if someone complains that we are doing bad things.

Note: given the hdfs rule that _ is the temp prefix I'm not over-worried, but I just think it may be good to have an emergency way to unwind

Change-Id: I9f31b531d637fb0da6f9cdd207e9628e7dcf5e88
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 1m 20s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 0m 0s test4tests The patch appears to include 10 new or modified test files.
_ trunk Compile Tests _
+0 🆗 mvndep 14m 12s Maven dependency ordering for branch
+1 💚 mvninstall 25m 9s trunk passed
+1 💚 compile 24m 6s trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 compile 19m 25s trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚 checkstyle 3m 9s trunk passed
+1 💚 mvnsite 2m 25s trunk passed
+1 💚 shadedclient 23m 17s branch has no errors when building and testing our client artifacts.
+1 💚 javadoc 1m 32s trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javadoc 2m 10s trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗 spotbugs 1m 10s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 3m 28s trunk passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 24s Maven dependency ordering for patch
+1 💚 mvninstall 1m 31s the patch passed
+1 💚 compile 21m 14s the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javac 21m 14s the patch passed
+1 💚 compile 18m 17s the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚 javac 18m 17s the patch passed
-0 ⚠️ checkstyle 2m 50s /diff-checkstyle-root.txt root: The patch generated 2 new + 51 unchanged - 1 fixed = 53 total (was 52)
+1 💚 mvnsite 2m 14s 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 17m 6s patch has no errors when building and testing our client artifacts.
+1 💚 javadoc 1m 25s the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚 javadoc 2m 9s the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
-1 ❌ findbugs 1m 21s /new-findbugs-hadoop-tools_hadoop-aws.html hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
_ Other Tests _
+1 💚 unit 17m 5s hadoop-common in the patch passed.
+1 💚 unit 1m 54s hadoop-aws in the patch passed.
+1 💚 asflicense 0m 50s The patch does not generate ASF License warnings.
210m 32s
Reason Tests
FindBugs module:hadoop-tools/hadoop-aws
Useless control flow in org.apache.hadoop.fs.s3a.commit.CommitUtils.getS3AFileSystem(Path, Configuration, boolean) At CommitUtils.java:Configuration, boolean) At CommitUtils.java:[line 91]
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2637/2/artifact/out/Dockerfile
GITHUB PR #2637
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle
uname Linux 59cf0131897a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 06a5d34
Default Java Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2637/2/testReport/
Max. process+thread count 1287 (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-2637/2/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
Copy link
Member

liuml07 commented Jan 26, 2021

Yes, this extra care is good to have I think. __magic might not be magic enough and it is possible (though unlikely) people use this path name for other purpose. I do not presume being able to disable this is definitely required; but it looks also good to me if it's possible.

@steveloughran
Copy link
Contributor Author

I agree. I will switch to on, leave the ability to disable off, and change in docs "how to disable" and explain what it means/why. I'll make a separate PR as it's very different from this one

@steveloughran
Copy link
Contributor Author

Closing, superceded by #2656

@steveloughran steveloughran deleted the s3/HADOOP-17483-magic-committer-always-supported branch October 15, 2021 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants