steveloughran commented on code in PR #5308:
URL: https://github.com/apache/hadoop/pull/5308#discussion_r1093469149
##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java:
##########
@@ -868,35 +892,41 @@ public void testDistCpUpdateCheckFileSkip() throws
Exception {
Path source = new Path(remoteDir, "file");
Path dest = new Path(localDir, "file");
+
+ Path source_0byte = new Path(remoteDir, "file_0byte");
+ Path dest_0byte = new Path(localDir, "file_0byte");
dest = localFS.makeQualified(dest);
+ dest_0byte = localFS.makeQualified(dest_0byte);
// Creating a source file with certain dataset.
byte[] sourceBlock = dataset(10, 'a', 'z');
// Write the dataset and as well create the target path.
- try (FSDataOutputStream out = remoteFS.create(source)) {
- out.write(sourceBlock);
- localFS.create(dest);
- }
+ ContractTestUtils.createFile(localFS, dest, true, sourceBlock);
+ ContractTestUtils
+ .writeDataset(remoteFS, source, sourceBlock, sourceBlock.length,
+ 1024, true);
- verifyPathExists(remoteFS, "", source);
- verifyPathExists(localFS, "", dest);
- DistCpTestUtils
- .assertRunDistCp(DistCpConstants.SUCCESS, remoteDir.toString(),
- localDir.toString(), "-delete -update" + getDefaultCLIOptions(),
- conf);
+ // Create 0 byte source and target files.
+ ContractTestUtils.createFile(remoteFS, source_0byte, true, new byte[0]);
+ ContractTestUtils.createFile(localFS, dest_0byte, true, new byte[0]);
+
+ // Execute the distcp -update job.
+ Job job = distCpUpdateWithFs(remoteDir, localDir, remoteFS, localFS);
// First distcp -update would normally copy the source to dest.
verifyFileContents(localFS, dest, sourceBlock);
+ // Verify 1 file was skipped in the distcp -update(They 0 byte files).
Review Comment:
nit: add space after update and replace They with `the`
##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java:
##########
@@ -50,6 +50,7 @@
import org.apache.hadoop.tools.mapred.CopyMapper;
import org.apache.hadoop.tools.util.DistCpTestUtils;
import org.apache.hadoop.util.functional.RemoteIterators;
+import org.apache.http.annotation.Contract;
Review Comment:
where does this come from/get used?
##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java:
##########
@@ -913,32 +943,65 @@ public void testDistCpUpdateCheckFileSkip() throws
Exception {
long newTargetModTimeNew = modTimeSourceUpd + MODIFICATION_TIME_OFFSET;
localFS.setTimes(dest, newTargetModTimeNew, -1);
- DistCpTestUtils
- .assertRunDistCp(DistCpConstants.SUCCESS, remoteDir.toString(),
- localDir.toString(), "-delete -update" + getDefaultCLIOptions(),
- conf);
+ // Execute the distcp -update job.
+ Job updatedSourceJobOldSrc =
+ distCpUpdateWithFs(remoteDir, localDir, remoteFS,
+ localFS);
// File contents should remain same since the mod time for target is
// newer than the updatedSource which indicates that the sync happened
// more recently and there is no update.
verifyFileContents(localFS, dest, sourceBlock);
+ // Skipped both 0 byte file and sourceFile(since mod time of target is
+ // older than the source it is perceived that source is of older version
+ // and we can skip it's copy).
+ verifySkipAndCopyCounter(updatedSourceJobOldSrc, 2, 0);
// Subtract by an offset which would ensure enough gap for the test to
// not fail due to race conditions.
long newTargetModTimeOld =
Math.min(modTimeSourceUpd - MODIFICATION_TIME_OFFSET, 0);
localFS.setTimes(dest, newTargetModTimeOld, -1);
- DistCpTestUtils
- .assertRunDistCp(DistCpConstants.SUCCESS, remoteDir.toString(),
- localDir.toString(), "-delete -update" + getDefaultCLIOptions(),
- conf);
+ // Execute the distcp -update job.
+ Job updatedSourceJobNewSrc = distCpUpdateWithFs(remoteDir, localDir,
+ remoteFS,
+ localFS);
- Assertions.assertThat(RemoteIterators.toList(localFS.listFiles(dest,
true)))
- .hasSize(1);
+ // Verifying the target directory have both 0 byte file and the content
+ // file.
+ Assertions
+ .assertThat(RemoteIterators.toList(localFS.listFiles(localDir, true)))
+ .hasSize(2);
// Now the copy should take place and the file contents should change
// since the mod time for target is older than the source file indicating
// that there was an update to the source after the last sync took place.
verifyFileContents(localFS, dest, updatedSourceBlock);
+ // Verifying we skipped the 0 byte file and copied the updated source
+ // file (since the modification time of the new source is older than the
+ // target now).
+ verifySkipAndCopyCounter(updatedSourceJobNewSrc, 1, 1);
Review Comment:
this is good; verifies the 0 byte support
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
