Copilot commented on code in PR #8858:
URL: https://github.com/apache/hadoop/pull/8858#discussion_r4373477854
##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java:
##########
@@ -252,6 +271,246 @@ protected Path getRemoteDir() {
return remoteDir;
}
+ @ParameterizedTest(name = "preserveTimes={0}, direct={1}, atomic={2}")
+ @CsvSource({
+ "true, false, false",
+ "false, false, false",
+ "true, true, false",
+ "false, true, false",
+ "true, false, true",
+ "false, false, true"
+ })
+ public void testChunkCopyPreserveTimes(boolean preserveTimes,
+ boolean directWrite, boolean atomicCommit) throws Exception {
+ requireChunkCopySupport();
+ Path source = new Path(remoteDir, "source");
+ Path target = new Path(remoteDir, "target");
+ Map<String, FileStatus> sourceStatuses = createChunkCopyTree(source);
+
+ DistCpOptions.Builder builder = chunkCopyOptions(source, target,
+ preserveTimes).withDirectWrite(directWrite || shouldUseDirectWrite())
+ .withAtomicCommit(atomicCommit);
+ runDistCp(builder.build(), true);
+
+ // The source directory itself is in the copy listing, so its times and
+ // those of nested directories must be restored after all concatenations.
+ verifyChunkCopyTree(target, sourceStatuses, preserveTimes, true);
+ }
+
+ @ParameterizedTest(name = "preserveTimes={0}, renameTarget={1}")
+ @CsvSource({"true, false", "false, false", "true, true", "false, true"})
+ public void testChunkCopyOutsideParentScope(boolean preserveTimes,
+ boolean renameTarget)
+ throws Exception {
+ requireChunkCopySupport();
+ Path sourceParent = new Path(remoteDir, "source-parent");
+ Path source = new Path(sourceParent, "file");
+ Path targetParent = new Path(remoteDir, "target-parent");
+ Path target = new Path(targetParent,
+ renameTarget ? "renamed-file" : source.getName());
+ createChunkCopyFile(source, TEST_FILE_LEN * 3 + 1);
+ FileStatus sourceStatus = setChunkCopyTimes(source, 0);
+ FileStatus sourceParentStatus = setChunkCopyTimes(sourceParent, 1);
+ mkdirs(remoteFS, targetParent);
+ FileStatus targetParentStatus = setChunkCopyTimes(targetParent, 2);
+
+ DistCpOptions.Builder builder = chunkCopyOptions(source,
+ renameTarget ? target : targetParent, preserveTimes);
+ if (renameTarget) {
+ builder.withDirectWrite(true);
+ }
+ runDistCp(builder.build(), true);
+
+ assertChunkCopyTimes(sourceStatus, remoteFS.getFileStatus(target),
+ preserveTimes);
+ // Creating/concatenating children may change the destination parent's
+ // mtime naturally. It must not inherit the source parent's timestamps.
+ FileStatus parentAfterCopy = remoteFS.getFileStatus(targetParent);
+ Assertions.assertThat(parentAfterCopy.getModificationTime())
+ .describedAs("mtime of parent outside copy scope")
+ .isNotEqualTo(sourceParentStatus.getModificationTime());
+ assertEquals(targetParentStatus.getAccessTime(),
+ parentAfterCopy.getAccessTime(), "atime of parent outside copy scope");
+ verifyFileContents(remoteFS, target,
+ dataset((int) sourceStatus.getLen(), 'a', 26));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testChunkCopyUpdateRootTimes(boolean updateRoot)
+ throws Exception {
+ requireChunkCopySupport();
+ Path source = new Path(remoteDir, "source");
+ Path target = new Path(remoteDir, "target");
+ Map<String, FileStatus> sourceStatuses = createChunkCopyTree(source);
+ // Force an existing destination file to be replaced by a chunked copy.
+ createFile(remoteFS, new Path(target, "large"), true, new byte[1]);
+ FileStatus targetRootStatus = setChunkCopyTimes(target, 20);
+ DistCpOptions options = chunkCopyOptions(source, target, true)
+ .withSyncFolder(true).withUpdateRoot(updateRoot).build();
+
+ runDistCp(options, true);
+ verifyChunkCopyTree(target, sourceStatuses, true, updateRoot);
+ if (!updateRoot) {
+ assertEquals(targetRootStatus.getAccessTime(),
+ remoteFS.getFileStatus(target).getAccessTime(),
+ "-update must not preserve root times without -updateRoot");
+ }
+
+ // A fully skipped update must still preserve file times without
+ // attempting to concatenate nonexistent chunk files.
+ setChunkCopyTimes(new Path(target, "large"), 30);
+ Job skipped = runDistCp(options, true);
+ assertCounterInRange(skipped, CopyMapper.Counter.COPY, 0, 0);
+ assertCounterInRange(skipped, CopyMapper.Counter.SKIP, 1, -1);
+ assertCounterInRange(skipped, CopyMapper.Counter.BYTESCOPIED, 0, 0);
+ verifyChunkCopyTree(target, sourceStatuses, true, updateRoot);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testChunkCopyPreserveOtherAttributes(boolean preserveTimes)
+ throws Exception {
+ requireChunkCopySupport();
+ skipIfUnsupported(SUPPORTS_UNIX_PERMISSIONS);
+ if (!remoteFS.hasPathCapability(remoteDir, FS_ACLS)
+ || !remoteFS.hasPathCapability(remoteDir, FS_XATTRS)) {
+ skip("ACL and XAttr support required");
+ }
+ Path source = new Path(remoteDir, "source");
+ Path target = new Path(remoteDir, "target");
+ Map<String, FileStatus> sourceStatuses = createChunkCopyTree(source);
+ Path sourceFile = new Path(source, "large");
+ remoteFS.setPermission(sourceFile, new FsPermission((short) 0640));
+ remoteFS.modifyAclEntries(sourceFile, Collections.singletonList(
+ new AclEntry.Builder().setScope(AclEntryScope.ACCESS)
+ .setType(AclEntryType.USER).setName("distcp-reader")
+ .setPermission(FsAction.READ).build()));
+ String xattrName = "user.distcp";
+ byte[] xattrValue = {1, 2, 3};
+ remoteFS.setXAttr(sourceFile, xattrName, xattrValue);
+ sourceStatuses.put("large", setChunkCopyTimes(sourceFile, 0));
+ sourceStatuses.put("", setChunkCopyTimes(source, 11));
+
+ runDistCp(chunkCopyOptions(source, target, preserveTimes)
+ .preserve("rbugpcax").build(), true);
+
+ // File listing entries omit extended ACLs: restoring all attributes from
+ // them after concat would erase ACLs already preserved by the mapper.
+ Path targetFile = new Path(target, "large");
+ assertEquals(remoteFS.getAclStatus(sourceFile).getEntries(),
+ remoteFS.getAclStatus(targetFile).getEntries(), "file ACL entries");
+ assertArrayEquals(xattrValue, remoteFS.getXAttr(targetFile, xattrName));
+ FileStatus expected = sourceStatuses.get("large");
+ FileStatus actual = remoteFS.getFileStatus(targetFile);
+ assertEquals(expected.getPermission(), actual.getPermission());
+ assertEquals(expected.getOwner(), actual.getOwner());
+ assertEquals(expected.getGroup(), actual.getGroup());
+ assertEquals(expected.getReplication(), actual.getReplication());
+ assertEquals(expected.getBlockSize(), actual.getBlockSize());
+ verifyChunkCopyTree(target, sourceStatuses, preserveTimes, true);
+ }
+
+ private void requireChunkCopySupport() throws IOException {
+ skipIfUnsupported(SUPPORTS_CONCAT);
+ skipIfUnsupported(SUPPORTS_SETTIMES);
+ skipIfUnsupported(SUPPORTS_BLOCK_LOCALITY);
Review Comment:
These checks admit `TestAdlContractDistCpLive`: `adls.xml` enables all three
capabilities, but ADL ignores the requested block size and reports a fixed 256
MiB block size. The new 3–4 KiB fixtures therefore fail the multi-block
assertion before DistCp runs on an otherwise working connector. Add a
fixture-support hook or contract capability to skip implementations that cannot
create these small multi-block files, or move these cases into the
HDFS-specific suite.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
