diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java index b2e521c695a..39b8a3fac75 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java @@ -482,8 +482,7 @@ public class DirectoryScanner implements Runnable { Collection diffRecord = new ArrayList<>(); statsRecord.totalBlocks = blockpoolReport.size(); - final List bl = dataset.getFinalizedBlocks(bpid); - Collections.sort(bl); // Sort based on blockId + final List bl = dataset.getSortedFinalizedBlocks(bpid); int d = 0; // index for blockpoolReport int m = 0; // index for memReprot diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java index 177c62e0174..89ad5103996 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java @@ -237,16 +237,17 @@ public interface FsDatasetSpi extends FSDatasetMBean { VolumeFailureSummary getVolumeFailureSummary(); /** - * Gets a list of references to the finalized blocks for the given block pool. + * Gets a sorted list of references to the finalized blocks for the given + * block pool. The list is sorted by blockID. *

* Callers of this function should call * {@link FsDatasetSpi#acquireDatasetLock} to avoid blocks' status being * changed during list iteration. *

* @return a list of references to the finalized blocks for the given block - * pool. + * pool. The list is sorted by blockID. */ - List getFinalizedBlocks(String bpid); + List getSortedFinalizedBlocks(String bpid); /** * Check whether the in-memory block record matches the block on the disk, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java index de898e93432..0ae08f6b038 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java @@ -1992,17 +1992,18 @@ class FsDatasetImpl implements FsDatasetSpi { } /** - * Gets a list of references to the finalized blocks for the given block pool. + * Gets a list of references to the finalized blocks for the given block pool, + * sorted by blockID. *

* Callers of this function should call * {@link FsDatasetSpi#acquireDatasetLock()} to avoid blocks' status being * changed during list iteration. *

* @return a list of references to the finalized blocks for the given block - * pool. + * pool. The list is sorted by blockID. */ @Override - public List getFinalizedBlocks(String bpid) { + public List getSortedFinalizedBlocks(String bpid) { try (AutoCloseableLock lock = datasetReadLock.acquire()) { final List finalized = new ArrayList( volumeMap.size(bpid)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestCrcCorruption.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestCrcCorruption.java index 917f0dbe093..df6a7dc8145 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestCrcCorruption.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestCrcCorruption.java @@ -173,7 +173,7 @@ public class TestCrcCorruption { final DataNode dn = cluster.getDataNodes().get(dnIdx); final String bpid = cluster.getNamesystem().getBlockPoolId(); List replicas = - dn.getFSDataset().getFinalizedBlocks(bpid); + dn.getFSDataset().getSortedFinalizedBlocks(bpid); assertTrue("Replicas do not exist", !replicas.isEmpty()); for (int idx = 0; idx < replicas.size(); idx++) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java index 6156c3da33c..65cf385f546 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReconstructStripedFile.java @@ -540,7 +540,7 @@ public class TestReconstructStripedFile { writeFile(fs, "/ec-xmits-weight", fileLen); DataNode dn = cluster.getDataNodes().get(0); - int corruptBlocks = dn.getFSDataset().getFinalizedBlocks( + int corruptBlocks = dn.getFSDataset().getSortedFinalizedBlocks( cluster.getNameNode().getNamesystem().getBlockPoolId()).size(); int expectedXmits = corruptBlocks * expectedWeight; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java index 8069b64246b..d9d7630c4f1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java @@ -1510,7 +1510,7 @@ public class SimulatedFSDataset implements FsDatasetSpi { } @Override - public List getFinalizedBlocks(String bpid) { + public List getSortedFinalizedBlocks(String bpid) { throw new UnsupportedOperationException(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/extdataset/ExternalDatasetImpl.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/extdataset/ExternalDatasetImpl.java index caaa89c25e0..13b740e08cf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/extdataset/ExternalDatasetImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/extdataset/ExternalDatasetImpl.java @@ -90,7 +90,7 @@ public class ExternalDatasetImpl implements FsDatasetSpi { } @Override - public List getFinalizedBlocks(String bpid) { + public List getSortedFinalizedBlocks(String bpid) { return null; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java index 626cceefea0..0e61220ed20 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java @@ -80,6 +80,7 @@ import java.io.OutputStreamWriter; import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.HashSet; import java.util.List; @@ -569,6 +570,41 @@ public class TestFsDatasetImpl { FsDatasetTestUtil.assertFileLockReleased(badDir.toString()); } + + @Test + /** + * This test is here primarily to catch any case where the datanode replica + * map structure is changed to a new structure which is not sorted and hence + * reading the blocks from it directly would not be sorted. + */ + public void testSortedFinalizedBlocksAreSorted() throws IOException { + this.conf = new HdfsConfiguration(); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); + try { + cluster.waitActive(); + DataNode dn = cluster.getDataNodes().get(0); + + FsDatasetSpi ds = DataNodeTestUtils.getFSDataset(dn); + ds.addBlockPool(BLOCKPOOL, conf); + + // Load 1000 blocks with random blockIDs + for (int i=0; i<=1000; i++) { + ExtendedBlock eb = new ExtendedBlock( + BLOCKPOOL, new Random().nextInt(), 1000, 1000 + i); + cluster.getFsDatasetTestUtils(0).createFinalizedReplica(eb); + } + // Get the sorted blocks and validate the arrayList is sorted + List replicaList = ds.getSortedFinalizedBlocks(BLOCKPOOL); + for (int i=0; i 0) { + // Not sorted so fail the test + fail("ArrayList is not sorted, and it should be"); + } + } + } finally { + cluster.shutdown(); + } + } @Test public void testDeletingBlocks() throws IOException {