Skip to content

Commit 62f0927

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

File tree

2 files changed

+110
-16
lines changed

2 files changed

+110
-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: 103 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,24 @@
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;
49+
import static org.mockito.Mockito.doReturn;
4350

4451
public class TestCorrectnessOfQuotaAfterRenameOp {
4552
private static MiniDFSCluster cluster;
@@ -193,7 +200,7 @@ public void testRenameWithoutValidFeature() throws Exception {
193200
QuotaUsage rootQuotaUsage1 = dfs.getQuotaUsage(new Path("/"));
194201
ContentSummary contentSummary1 = dfs.getContentSummary(testParentDir1);
195202
// srcDir=/testRename/testDir1/src-dir
196-
// dstDir=/testRename/testDir2/dst-dir dstDir1 not exist
203+
// dstDir=/testRename/testDir2/dst-dir dstDir not exist
197204
final Path dstDir2 = new Path(testParentDir2, "dst-dir");
198205
assertTrue(dfs.rename(srcDir, dstDir2));
199206
ContentSummary contentSummary2 = dfs.getContentSummary(testParentDir2);
@@ -208,19 +215,106 @@ public void testRenameWithoutValidFeature() throws Exception {
208215
// 2. Test rename2
209216
final Path dstDir3 = new Path(testParentDir3, "dst-dir");
210217
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;
218+
long originDstDirUsage = dfs.getQuotaUsage(dstDir3).getFileAndDirectoryCount();
219+
// Overwrite the rename destination, the usage of dstDir3 should be excluded
220+
long expectedCount =
221+
dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount() - originDstDirUsage;
215222
ContentSummary contentSummary3 = dfs.getContentSummary(testParentDir2);
216223

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

0 commit comments

Comments
 (0)