Skip to content

HDFS-17045. File renamed from a snapshottable dir to a non-snapshottable dir cannot be deleted. #5738

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

Merged
szetszwo merged 6 commits into from
Jun 14, 2023
Merged

HDFS-17045. File renamed from a snapshottable dir to a non-snapshottable dir cannot be deleted. #5738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
271233a
HDFS-17045. File renamed from a snapshottable dir to a non-snapshotta…
szetszwo Jun 12, 2023
9600756
Trigger hadoop-yetus.
szetszwo Jun 13, 2023
70e3977
Trigger unit tests.
szetszwo Jun 13, 2023
30ab7c8
Add assertions for rename within the snapshot directory.
szetszwo Jun 14, 2023
a2c05a3
trigger build
szetszwo Jun 14, 2023
bc79520
trigger build again
szetszwo Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions ...ject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,17 +831,17 @@ public void cleanSubtree(ReclaimContext reclaimContext, int snapshot,
*
* @param context to {@link ReclaimContext#getSnapshotIdToBeDeleted()}
*/
private boolean shouldDestroy(ReclaimContext context) {
private void shouldDestroy(ReclaimContext context) {
final int snapshotToBeDeleted = context.getSnapshotIdToBeDeleted();
if (dstSnapshotId < snapshotToBeDeleted) {
return true;
if (snapshotToBeDeleted == Snapshot.CURRENT_STATE_ID
|| snapshotToBeDeleted > dstSnapshotId) {
return;
}
LOG.warn("Try to destroy a DstReference with dstSnapshotId = {}"
+ " >= snapshotToBeDeleted = {}", dstSnapshotId, snapshotToBeDeleted);
LOG.warn(" dstRef: {}", toDetailString());
final INode r = getReferredINode().asReference().getReferredINode();
LOG.warn(" referred: {}", r.toDetailString());
return false;
}

/**
Expand All @@ -857,9 +857,7 @@ private boolean shouldDestroy(ReclaimContext context) {
*/
@Override
public void destroyAndCollectBlocks(ReclaimContext reclaimContext) {
if (!shouldDestroy(reclaimContext)) {
return;
}
shouldDestroy(reclaimContext);

// since we count everything of the subtree for the quota usage of a
// dst reference node, here we should just simply do a quota computation.
Expand Down
3 changes: 3 additions & 0 deletions ...rc/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public void testRenameFromSDir2NonSDir() throws Exception {

hdfs.delete(bar, false);
Assert.assertEquals(1, withCount.getReferenceCount());
restartClusterAndCheckImage(true);
}

private static boolean existsInDiffReport(List<DiffReportEntry> entries,
Expand Down Expand Up @@ -196,6 +197,7 @@ public void testRenameFileNotInSnapshot() throws Exception {
assertTrue(existsInDiffReport(entries, DiffType.MODIFY, "", null));
assertTrue(existsInDiffReport(entries, DiffType.CREATE, file2.getName(),
null));
restartClusterAndCheckImage(true);
}

/**
Expand All @@ -218,6 +220,7 @@ public void testRenameFileInSnapshot() throws Exception {
assertTrue(existsInDiffReport(entries, DiffType.MODIFY, "", null));
assertTrue(existsInDiffReport(entries, DiffType.RENAME, file1.getName(),
file2.getName()));
restartClusterAndCheckImage(true);
}

@Test (timeout=60000)
Expand Down