HADOOP-17122: Preserving Directory Attributes in DistCp with Atomic Copy #2133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java
Show resolved
Hide resolved
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java
Outdated
Show resolved
Hide resolved
|
Looks good from my side. Though there is one checkstyle issue. Please address that. |
|
@steveloughran Please take a look at this PR. |
|
I will review this on my friday afternoon PR review session. |
|
Sure Thanks |
|
@steveloughran Did you take a look at this? |
steveloughran
requested changes
Aug 10, 2020
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java
Show resolved
Hide resolved
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java
Show resolved
Hide resolved
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/mapred/TestCopyCommitter.java
Outdated
Show resolved
Hide resolved
|
@swamirishi while you are looking at distcp, have a look at #1111 and see what you think about it...its test code is pretty close to yours, the functionality orthoganal |
swamirishi
force-pushed
the
HADOOP-17122
branch
from
91f2f87 to
1dc17b5
Compare
@steveloughran The testcase in the aforementioned PR is not testing for Atomic copy. It would not matter for this particular code flow as the missing deletion from target FileSystem would be always happening over the CONF_LABEL_TARGET_WORK_PATH & not over the CONF_LABEL_TARGET_FINAL_PATH. So in this code flow one need not test for Atomic Copy case where Work Path & Final Path would be different |
|
💔 -1 overall
This message was automatically generated. |
swamirishi
force-pushed
the
HADOOP-17122
branch
from
1dc17b5 to
166ec38
Compare
|
💔 -1 overall
This message was automatically generated. |
|
hey, no need to rebase unless there's a change in the hadoop source which creates merge problems. Github reviews work better if the PRs are left alone, so that it's possible to tie up discussions with specific commits. Something to bear in mind on your next PR.... |
steveloughran
requested changes
Aug 12, 2020
| @@ -650,4 +653,4 @@ public RecordReader createRecordReader(InputSplit split, | |||
| return null; | |||
| } | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
nit: restore the line ending
|
@steveloughran I have removed the whitespace from the new line I added. Can we get the code merged? |
|
💔 -1 overall
This message was automatically generated. |
|
@steveloughran I have made the required changes. Can the code be merged? |
|
@steveloughran I have made the required changes to fix checksum issue. It worked on local test-patch. Can the code be merged? |
|
🎊 +1 overall
This message was automatically generated. |
|
@steveloughran I have received a +1. There are no issues. Can you merge the code? |
|
I'm on holiday this week. Don't panic. will look at when I return. I am not reviewing any patches this week |
asfgit
pushed a commit
that referenced
this pull request
Aug 22, 2020
…opy (#2133) Contributed by Swaminathan Balachandran Change-Id: I86f956dd4ab0b278d923fe7b70037e6b929a8aa1
|
+1. Patch merged to 3.3+,. If you want it in -3.2 then create a separate PR against branch-3.2 with this patch applied and submit it, just to see what Yetus says. We won't be doing any more actual code reviews, just having it regression test the backport. thanks! |
swaminathan-b
pushed a commit
to swaminathan-b/hadoop
that referenced
this pull request
Aug 22, 2020
…opy (apache#2133) Contributed by Swaminathan Balachandran
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 23, 2023
…opy (apache#2133) Contributed by Swaminathan Balachandran Ref.: ENGESC-7613 (cherry picked from commit 872c290) Change-Id: I39a299a051b285e2f655b2cac6dea6f6c58ec21b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute