smarthanwang commented on code in PR #6464:
URL: https://github.com/apache/hadoop/pull/6464#discussion_r1458970508
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java:
##########
@@ -2011,4 +2011,83 @@ public void tesInvalidateMissingBlock() throws Exception
{
cluster.shutdown();
}
}
+
+ @Test
+ public void testCheckFilesWhenInvalidateMissingBlock() throws Exception {
+ long blockSize = 1024;
+ int heartbeatInterval = 1;
+ HdfsConfiguration c = new HdfsConfiguration();
+ c.setInt(DFSConfigKeys.DFS_HEARTBEAT_INTERVAL_KEY, heartbeatInterval);
+ c.setLong(DFS_BLOCK_SIZE_KEY, blockSize);
+ MiniDFSCluster cluster = new MiniDFSCluster.Builder(c).
+ numDataNodes(1).build();
+ DataNodeFaultInjector oldDnInjector = DataNodeFaultInjector.get();
+ try {
+ cluster.waitActive();
+ BlockReaderTestUtil util = new BlockReaderTestUtil(cluster, new
+ HdfsConfiguration(conf));
+ Path path = new Path("/testFile");
+ util.writeFile(path, 1);
+ String bpid = cluster.getNameNode().getNamesystem().getBlockPoolId();
+ DataNode dn = cluster.getDataNodes().get(0);
+ FsDatasetImpl dnFSDataset = (FsDatasetImpl) dn.getFSDataset();
+ List<ReplicaInfo> replicaInfos = dnFSDataset.getFinalizedBlocks(bpid);
+ assertEquals(1, replicaInfos.size());
+ DFSTestUtil.readFile(cluster.getFileSystem(), path);
+ LocatedBlock blk = util.getFileBlocks(path, 512).get(0);
+ ExtendedBlock block = blk.getBlock();
+
+ // Append a new block with an incremented generation stamp.
+ long newGS = block.getGenerationStamp() + 1;
+ dnFSDataset.append(block, newGS, 1024);
+ block.setGenerationStamp(newGS);
+
+ DataNodeFaultInjector injector = new DataNodeFaultInjector() {
+ @Override
+ public void delayGetMetaDataInputStream() {
+ try {
+ Thread.sleep(8000);
+ } catch (InterruptedException e) {
+ // Ignore exception.
+ }
+ }
+ };
+ // Delay to getMetaDataInputStream.
+ DataNodeFaultInjector.set(injector);
+
+ ExecutorService executorService = Executors.newFixedThreadPool(2);
+ try {
+ Future<?> blockReaderFuture = executorService.submit(() -> {
+ try {
+ // Submit tasks for reading block.
+ BlockReaderTestUtil.getBlockReader(cluster.getFileSystem(), blk,
0, 512);
Review Comment:
Shoud we check FNE's thrown as expect here?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java:
##########
@@ -2416,11 +2419,21 @@ public void invalidateMissingBlock(String bpid, Block
block) {
// So remove if from volume map notify namenode is ok.
try (AutoCloseableLock lock = lockManager.writeLock(LockLevel.BLOCK_POOl,
bpid)) {
- ReplicaInfo replica = volumeMap.remove(bpid, block);
- invalidate(bpid, replica);
+ // Check if this block is on the volume map.
+ ReplicaInfo replica = volumeMap.get(bpid, block);
+ // Double-check block or meta file existence when checkFiles as true.
+ if (replica != null && (!checkFiles ||
+ (!replica.blockDataExists() || !replica.metadataExists()))) {
+ volumeMap.remove(bpid, block);
+ invalidate(bpid, replica);
Review Comment:
If replica == null，` invalidate(bpid, replica);` would not execute
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java:
##########
@@ -2011,4 +2011,83 @@ public void tesInvalidateMissingBlock() throws Exception
{
cluster.shutdown();
}
}
+
+ @Test
+ public void testCheckFilesWhenInvalidateMissingBlock() throws Exception {
+ long blockSize = 1024;
+ int heartbeatInterval = 1;
+ HdfsConfiguration c = new HdfsConfiguration();
+ c.setInt(DFSConfigKeys.DFS_HEARTBEAT_INTERVAL_KEY, heartbeatInterval);
+ c.setLong(DFS_BLOCK_SIZE_KEY, blockSize);
+ MiniDFSCluster cluster = new MiniDFSCluster.Builder(c).
+ numDataNodes(1).build();
+ DataNodeFaultInjector oldDnInjector = DataNodeFaultInjector.get();
+ try {
+ cluster.waitActive();
+ BlockReaderTestUtil util = new BlockReaderTestUtil(cluster, new
+ HdfsConfiguration(conf));
+ Path path = new Path("/testFile");
+ util.writeFile(path, 1);
+ String bpid = cluster.getNameNode().getNamesystem().getBlockPoolId();
+ DataNode dn = cluster.getDataNodes().get(0);
+ FsDatasetImpl dnFSDataset = (FsDatasetImpl) dn.getFSDataset();
+ List<ReplicaInfo> replicaInfos = dnFSDataset.getFinalizedBlocks(bpid);
+ assertEquals(1, replicaInfos.size());
+ DFSTestUtil.readFile(cluster.getFileSystem(), path);
+ LocatedBlock blk = util.getFileBlocks(path, 512).get(0);
+ ExtendedBlock block = blk.getBlock();
+
+ // Append a new block with an incremented generation stamp.
+ long newGS = block.getGenerationStamp() + 1;
+ dnFSDataset.append(block, newGS, 1024);
+ block.setGenerationStamp(newGS);
+
+ DataNodeFaultInjector injector = new DataNodeFaultInjector() {
+ @Override
+ public void delayGetMetaDataInputStream() {
+ try {
+ Thread.sleep(8000);
+ } catch (InterruptedException e) {
+ // Ignore exception.
+ }
+ }
+ };
+ // Delay to getMetaDataInputStream.
+ DataNodeFaultInjector.set(injector);
+
+ ExecutorService executorService = Executors.newFixedThreadPool(2);
+ try {
+ Future<?> blockReaderFuture = executorService.submit(() -> {
+ try {
+ // Submit tasks for reading block.
+ BlockReaderTestUtil.getBlockReader(cluster.getFileSystem(), blk,
0, 512);
+ } catch (IOException e) {
+ // Ignore exception.
+ }
+ });
+
+ Future<?> finalizeBlockFuture = executorService.submit(() -> {
+ try {
+ // Submit tasks for finalizing block.
+ Thread.sleep(1000);
+ dnFSDataset.finalizeBlock(block, false);
+ } catch (Exception e) {
+ // Ignore exception
+ }
+ });
+
+ // Wait for both tasks to complete.
+ blockReaderFuture.get();
+ finalizeBlockFuture.get();
+ } finally {
+ executorService.shutdown();
+ }
+
+ // Validate the replica is exits.
+ assertNotNull(dnFSDataset.getReplicaInfo(blk.getBlock()));
Review Comment:
I guess we need one more case to check the `ReplicaInfo` and `data file`
would lost when` checkFile=false`
--
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]
