Skip to content

HDFS-9913. DistCp to add -useTrash to move deleted files to Trash #974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

HDFS-9913. DistCp to add -useTrash to move deleted files to Trash #974

wants to merge 3 commits into from

Conversation

asagjj
Copy link
Contributor

@asagjj asagjj commented Jun 15, 2019

No description provided.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 75 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 5 new or modified test files.
_ trunk Compile Tests _
+1 mvninstall 1327 trunk passed
+1 compile 28 trunk passed
+1 checkstyle 21 trunk passed
+1 mvnsite 30 trunk passed
+1 shadedclient 787 branch has no errors when building and testing our client artifacts.
+1 javadoc 21 trunk passed
0 spotbugs 45 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 43 trunk passed
_ Patch Compile Tests _
+1 mvninstall 25 the patch passed
+1 compile 23 the patch passed
+1 javac 23 the patch passed
-0 checkstyle 16 hadoop-tools/hadoop-distcp: The patch generated 6 new + 136 unchanged - 0 fixed = 142 total (was 136)
+1 mvnsite 26 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 828 patch has no errors when building and testing our client artifacts.
+1 javadoc 19 the patch passed
+1 findbugs 51 the patch passed
_ Other Tests _
-1 unit 834 hadoop-distcp in the patch failed.
+1 asflicense 28 The patch does not generate ASF License warnings.
4297
Reason Tests
Failed junit tests hadoop.tools.contract.TestLocalContractDistCp
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/hadoop-multibranch/job/PR-974/1/artifact/out/Dockerfile
GITHUB PR #974
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux cf29b11bc491 4.4.0-141-generic #167~14.04.1-Ubuntu SMP Mon Dec 10 13:20:24 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / 0766186
Default Java 1.8.0_212
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-974/1/artifact/out/diff-checkstyle-hadoop-tools_hadoop-distcp.txt
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-974/1/artifact/out/patch-unit-hadoop-tools_hadoop-distcp.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-974/1/testReport/
Max. process+thread count 305 (vs. ulimit of 5500)
modules C: hadoop-tools/hadoop-distcp U: hadoop-tools/hadoop-distcp
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-974/1/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
0 reexec 75 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 5 new or modified test files.
_ trunk Compile Tests _
+1 mvninstall 1238 trunk passed
+1 compile 29 trunk passed
+1 checkstyle 21 trunk passed
+1 mvnsite 31 trunk passed
+1 shadedclient 774 branch has no errors when building and testing our client artifacts.
+1 javadoc 21 trunk passed
0 spotbugs 46 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 42 trunk passed
_ Patch Compile Tests _
+1 mvninstall 27 the patch passed
+1 compile 24 the patch passed
+1 javac 24 the patch passed
-0 checkstyle 16 hadoop-tools/hadoop-distcp: The patch generated 1 new + 136 unchanged - 0 fixed = 137 total (was 136)
+1 mvnsite 26 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 811 patch has no errors when building and testing our client artifacts.
+1 javadoc 20 the patch passed
+1 findbugs 49 the patch passed
_ Other Tests _
+1 unit 892 hadoop-distcp in the patch passed.
+1 asflicense 34 The patch does not generate ASF License warnings.
4232
Subsystem Report/Notes
Docker Client=18.09.5 Server=18.09.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-974/2/artifact/out/Dockerfile
GITHUB PR #974
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 46a9b8f0b152 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3 08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / 304a47e
Default Java 1.8.0_212
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-974/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-distcp.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-974/2/testReport/
Max. process+thread count 342 (vs. ulimit of 5500)
modules C: hadoop-tools/hadoop-distcp U: hadoop-tools/hadoop-distcp
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-974/2/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@asagjj
Copy link
Contributor Author

asagjj commented Jun 18, 2019

@steveloughran @jojochuang Could you please help to review this commit?

steveloughran
steveloughran reviewed Jul 11, 2019
View reviewed changes
@steveloughran
Copy link
Contributor

Production code looks good other than a few minor issues; commented more on the tests.

Have you run any of the object store tests runs (abfs, s3a, etc.)? Not a blocker but I'd prefer this, as otherwise I'll end up doing it

@asagjj
Copy link
Contributor Author

asagjj commented Jul 11, 2019

Thanks @steveloughran for patient review!! I had ran and passed all mentioned object stores(abfs, azure, s3a,adl, aliyun) unit tests. I will update commits according to you suggestions soon.

@steveloughran steveloughran changed the title HDFS-9913.DispCp doesn't use Trash with -delete option HDFS-9913. DistCp doesn't use Trash with -delete option Jul 17, 2019
@steveloughran
Copy link
Contributor

OK, +1 from me

@steveloughran steveloughran changed the title HDFS-9913. DistCp doesn't use Trash with -delete option HDFS-9913. DistCp to add -useTrash to move deleted files to Trash Jul 17, 2019
@steveloughran
Copy link
Contributor

committed; extracted your email address from the .patch file so that you are credited as the author of the patch.

@steveloughran
Copy link
Contributor

Update: JIRA playing up so can't close the issue.

But I'm also getting a failure on my test run

[INFO] Running org.apache.hadoop.fs.s3a.scale.ITestS3AConcurrentOps
[ERROR] Tests run: 9, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 223.958 s <<< FAILURE! - in org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp
[ERROR] testUpdateUseTrashDeepDirectoryStructureToRemote(org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp)  Time elapsed: 20.398 s  <<< ERROR!
java.io.FileNotFoundException: about to be deleted file: not found s3a://hwdev-steve-ireland-new/user/stevel/.Trash in s3a://hwdev-steve-ireland-new/user/stevel
	at org.apache.hadoop.fs.contract.ContractTestUtils.verifyPathExists(ContractTestUtils.java:942)
	at org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists(ContractTestUtils.java:920)
	at org.apache.hadoop.fs.contract.ContractTestUtils.assertDeleted(ContractTestUtils.java:704)
	at org.apache.hadoop.fs.contract.ContractTestUtils.assertDeleted(ContractTestUtils.java:686)
	at org.apache.hadoop.tools.contract.AbstractContractDistCpTest.distCpUpdateUseTrashDeepDirectoryStructure(AbstractContractDistCpTest.java:358)
	at org.apache.hadoop.tools.contract.AbstractContractDistCpTest.testUpdateUseTrashDeepDirectoryStructureToRemote(AbstractContractDistCpTest.java:233)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.io.FileNotFoundException: No such file or directory: s3a://hwdev-steve-ireland-new/user/stevel/.Trash
	at org.apache.hadoop.fs.s3a.S3AFileSystem.s3GetFileStatus(S3AFileSystem.java:2837)
	at org.apache.hadoop.fs.s3a.S3AFileSystem.innerGetFileStatus(S3AFileSystem.java:2726)
	at org.apache.hadoop.fs.s3a.S3AFileSystem.getFileStatus(S3AFileSystem.java:2620)
	at org.apache.hadoop.fs.contract.ContractTestUtils.verifyPathExists(ContractTestUtils.java:936)
	... 21 more

I'll look at this a bit more and decide whether we should fix this with rollback vs another iteration of the PR

@asagjj asagjj deleted the HDFS-9913 branch July 17, 2019 12:05
@steveloughran
Copy link
Contributor

OK, cause is that the test setup calls assertDeleted() to clean up the trash dir, and the CTU check there actually fails if the file doesn't exist before the delete. Which is actually something we should question.

For now: reverted this PR so everything will be working before the US wakes up. I know it's a quick fix but I'd be depending on others to review, etc, etc and the failing test will be around for 24h minimum.

@asagjj : Can you submit an updated PR with the delete setup something like:

    Path trashRootDir = remoteFS.getTrashRoot(null);
    remoteFS.delete(trashRootDir, true);
    ContractTestUtils.assertPathDoesNotExist(remoteFS, "trash",  trashRootDir )

We don't need to worry about the before state or the return value of delete, just that the dir isn't there.
Thanks

@asagjj asagjj restored the HDFS-9913 branch July 17, 2019 12:29
@asagjj
Copy link
Contributor Author

asagjj commented Jul 17, 2019 •
edited
Loading

@steveloughran Sorry about that, I didn't reproduce this failure in my enviroment. And since this pr is closed ,I open a new one to update the commit #1111

@steveloughran
Copy link
Contributor

thx, I'll look at it tomorrow. I'll run the tests first just to make sure all is good.

shanthoosh added a commit to shanthoosh/hadoop that referenced this pull request Oct 15, 2019
@shanthoosh
SAMZA-2147: Improve the documentation of the samza release process. (a…
a74b500 
…pache#974)

* Improve thedocumentation of the samza release process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steveloughran steveloughran steveloughran left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@asagjj @hadoop-yetus @steveloughran