Skip to content

Commit 1e9321b

Browse files
huangzhaobo99huangzhaobo99
committed
fix update quota & add ut
1 parent 3c60bf0 commit 1e9321b

File tree

2 files changed

+109
-16
lines changed

2 files changed

+109
-16
lines changed

‎hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,8 +1398,9 @@ public INodesInPath addLastINode(INodesInPath existing, INode inode, FsPermissio
13981398
// always verify inode name
13991399
verifyINodeName(inode.getLocalNameBytes());
14001400

1401+
QuotaCounts counts = null;
14011402
if (updateQuota) {
1402-
QuotaCounts counts = quotaCount.orElseGet(() -> inode.
1403+
counts = quotaCount.orElseGet(() -> inode.
14031404
computeQuotaUsage(getBlockStoragePolicySuite(),
14041405
parent.getStoragePolicyID(), false,
14051406
Snapshot.CURRENT_STATE_ID));
@@ -1409,12 +1410,11 @@ public INodesInPath addLastINode(INodesInPath existing, INode inode, FsPermissio
14091410
boolean isRename = (inode.getParent() != null);
14101411
final boolean added = parent.addChild(inode, true,
14111412
existing.getLatestSnapshotId());
1412-
if (!added && updateQuota) {
1413-
QuotaCounts counts = quotaCount.orElseGet(() -> inode.
1414-
computeQuotaUsage(getBlockStoragePolicySuite(),
1415-
parent.getStoragePolicyID(), false,
1416-
Snapshot.CURRENT_STATE_ID));
1417-
updateCountNoQuotaCheck(existing, pos, counts.negation());
1413+
if (!added) {
1414+
// When adding INode fails, if 'updateQuota' is true, rollback quota.
1415+
if (updateQuota) {
1416+
updateCountNoQuotaCheck(existing, pos, counts.negation());
1417+
}
14181418
return null;
14191419
} else {
14201420
if (!isRename) {

‎hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCorrectnessOfQuotaAfterRenameOp.java‎

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,23 @@
2929
import org.apache.hadoop.hdfs.protocol.HdfsConstants;
3030
import org.apache.hadoop.hdfs.server.blockmanagement.BlockStoragePolicySuite;
3131
import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
32+
import org.apache.hadoop.ipc.RemoteException;
3233
import org.apache.hadoop.test.GenericTestUtils;
3334
import org.apache.hadoop.test.PathUtils;
3435
import org.junit.jupiter.api.BeforeAll;
3536
import org.junit.jupiter.api.Test;
37+
import org.mockito.Mockito;
3638

3739
import java.io.IOException;
3840

3941
import static org.apache.hadoop.hdfs.protocol.HdfsConstants.HOT_STORAGE_POLICY_NAME;
4042
import static org.apache.hadoop.hdfs.protocol.HdfsConstants.ONESSD_STORAGE_POLICY_NAME;
4143
import static org.junit.jupiter.api.Assertions.assertEquals;
44+
import static org.junit.jupiter.api.Assertions.assertFalse;
45+
import static org.junit.jupiter.api.Assertions.assertThrows;
4246
import static org.junit.jupiter.api.Assertions.assertTrue;
47+
import static org.mockito.ArgumentMatchers.anyBoolean;
48+
import static org.mockito.ArgumentMatchers.anyInt;
4349

4450
public class TestCorrectnessOfQuotaAfterRenameOp {
4551
private static MiniDFSCluster cluster;
@@ -193,7 +199,7 @@ public void testRenameWithoutValidFeature() throws Exception {
193199
QuotaUsage rootQuotaUsage1 = dfs.getQuotaUsage(new Path("/"));
194200
ContentSummary contentSummary1 = dfs.getContentSummary(testParentDir1);
195201
// srcDir=/testRename/testDir1/src-dir
196-
// dstDir=/testRename/testDir2/dst-dir dstDir1 not exist
202+
// dstDir=/testRename/testDir2/dst-dir dstDir not exist
197203
final Path dstDir2 = new Path(testParentDir2, "dst-dir");
198204
assertTrue(dfs.rename(srcDir, dstDir2));
199205
ContentSummary contentSummary2 = dfs.getContentSummary(testParentDir2);
@@ -208,19 +214,106 @@ public void testRenameWithoutValidFeature() throws Exception {
208214
// 2. Test rename2
209215
final Path dstDir3 = new Path(testParentDir3, "dst-dir");
210216
assertTrue(dfs.mkdirs(dstDir3));
211-
long originDstDir2Usage = dfs.getQuotaUsage(dstDir3).getFileAndDirectoryCount();
212-
// Exclude dstDir2 usage
213-
long rootINodeCount1 =
214-
dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount() - originDstDir2Usage;
217+
long originDstDirUsage = dfs.getQuotaUsage(dstDir3).getFileAndDirectoryCount();
218+
// Overwrite the rename destination, the usage of dstDir3 should be excluded
219+
long expectedCount =
220+
dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount() - originDstDirUsage;
215221
ContentSummary contentSummary3 = dfs.getContentSummary(testParentDir2);
216222

217-
// Src and dst must be same (all file or all dir)
218-
// dstDir2=/testRename/testDir3/dst-dir
223+
// Src and dst must be same
224+
// dstDir2=/testRename/testDir2/dst-dir
219225
// dstDir3=/testRename/testDir3/dst-dir
220226
dfs.rename(dstDir2, dstDir3, Options.Rename.OVERWRITE);
221-
long rootINodeCount2 = dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount();
222-
assertEquals(rootINodeCount1, rootINodeCount2);
227+
long actualCount = dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount();
228+
assertEquals(expectedCount, actualCount);
223229
ContentSummary contentSummary4 = dfs.getContentSummary(testParentDir3);
224230
assertEquals(contentSummary3, contentSummary4);
225231
}
232+
233+
@Test
234+
public void testRenameUndoWithoutValidFeature() throws Exception {
235+
final int fileLen = 1024;
236+
final short replication = 3;
237+
final Path root = new Path("/testRenameUndo");
238+
assertTrue(dfs.mkdirs(root));
239+
240+
Path testParentDir1 = new Path(root, "testDir1");
241+
assertTrue(dfs.mkdirs(testParentDir1));
242+
Path testParentDir2 = new Path(root, "testDir2");
243+
assertTrue(dfs.mkdirs(testParentDir2));
244+
Path testParentDir3 = new Path(root, "testDir3");
245+
assertTrue(dfs.mkdirs(testParentDir3));
246+
Path testParentDir4 = new Path(root, "testDir4");
247+
assertTrue(dfs.mkdirs(testParentDir4));
248+
249+
final Path srcDir1 = new Path(testParentDir1, "src-dir");
250+
for (int i = 0; i < 2; i++) {
251+
Path file1 = new Path(srcDir1, "file" + i);
252+
DFSTestUtil.createFile(dfs, file1, fileLen, replication, 0);
253+
}
254+
255+
final Path srcDir3 = new Path(testParentDir3, "src-dir");
256+
for (int i = 0; i < 2; i++) {
257+
Path file1 = new Path(srcDir3, "file" + i);
258+
DFSTestUtil.createFile(dfs, file1, fileLen, replication, 0);
259+
}
260+
261+
// Test rename1
262+
ContentSummary rootContentSummary1 = dfs.getContentSummary(new Path("/"));
263+
QuotaUsage rootQuotaUsage1 = dfs.getQuotaUsage(new Path("/"));
264+
ContentSummary contentSummary1 = dfs.getContentSummary(testParentDir1);
265+
266+
FSNamesystem fsn = cluster.getNamesystem();
267+
FSDirectory fsDirectory = fsn.getFSDirectory();
268+
269+
// Replace INode, expected addChild return false
270+
INodeDirectory dir = fsDirectory.getINode4Write(testParentDir2.toString()).asDirectory();
271+
INodeDirectory mockDir = Mockito.spy(dir);
272+
INode srcInode = fsDirectory.getINode(srcDir3.toString());
273+
// Fail the rename but succeed in undo
274+
Mockito.doReturn(false).when(mockDir).addChild(Mockito.eq(srcInode), anyBoolean(), anyInt());
275+
INodeDirectory rootDir = fsDirectory.getINode4Write(root.toString()).asDirectory();
276+
rootDir.replaceChild(dir, mockDir, fsDirectory.getINodeMap());
277+
mockDir.setParent(rootDir);
278+
279+
// srcDir=/testRenameUndo/testDir1/src-dir
280+
// dstDir=/testRenameUndo/testDir2/
281+
assertFalse(dfs.rename(srcDir3, testParentDir2));
282+
283+
ContentSummary rootContentSummary2 = dfs.getContentSummary(new Path("/"));
284+
QuotaUsage rootQuotaUsage2 = dfs.getQuotaUsage(new Path("/"));
285+
ContentSummary contentSummary2 = dfs.getContentSummary(testParentDir1);
286+
assertEquals(rootContentSummary1.toString(), rootContentSummary2.toString());
287+
assertEquals(rootQuotaUsage1.toString(), rootQuotaUsage2.toString());
288+
assertEquals(contentSummary1.toString(), contentSummary2.toString());
289+
assertEquals(rootContentSummary1.getFileAndDirectoryCount(),
290+
rootQuotaUsage2.getFileAndDirectoryCount());
291+
292+
// Test rename2
293+
final Path dstDir4 = new Path(testParentDir4, "src-dir");
294+
assertTrue(dfs.mkdirs(dstDir4));
295+
ContentSummary rootContentSummary3 = dfs.getContentSummary(new Path("/"));
296+
QuotaUsage rootQuotaUsage3 = dfs.getQuotaUsage(new Path("/"));
297+
ContentSummary contentSummary3 = dfs.getContentSummary(testParentDir3);
298+
299+
// Replace INode, expected addChild return false
300+
INodeDirectory dir4 = fsDirectory.getINode4Write(testParentDir4.toString()).asDirectory();
301+
INodeDirectory mockDir4 = Mockito.spy(dir4);
302+
INode srcInode3 = fsDirectory.getINode(srcDir3.toString());
303+
Mockito.doReturn(false).when(mockDir4).addChild(Mockito.eq(srcInode3), anyBoolean(), anyInt());
304+
rootDir.replaceChild(dir4, mockDir4, fsDirectory.getINodeMap());
305+
mockDir4.setParent(rootDir);
306+
// srcDir=/testRenameUndo/testDir3/src-dir
307+
// dstDir=/testRenameUndo/testDir4/src-dir dstDir exist
308+
assertThrows(RemoteException.class,
309+
() -> dfs.rename(srcDir3, dstDir4, Options.Rename.OVERWRITE));
310+
ContentSummary rootContentSummary4 = dfs.getContentSummary(new Path("/"));
311+
QuotaUsage rootQuotaUsage4 = dfs.getQuotaUsage(new Path("/"));
312+
ContentSummary contentSummary4 = dfs.getContentSummary(testParentDir3);
313+
assertEquals(rootContentSummary3.toString(), rootContentSummary4.toString());
314+
assertEquals(rootQuotaUsage3.toString(), rootQuotaUsage4.toString());
315+
assertEquals(contentSummary3.toString(), contentSummary4.toString());
316+
assertEquals(rootContentSummary3.getFileAndDirectoryCount(),
317+
rootQuotaUsage4.getFileAndDirectoryCount());
318+
}
226319
}

0 commit comments

Comments
 (0)