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 99584d9f3c3..aede03e6164 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 @@ -22,7 +22,6 @@ import java.io.File; import java.io.FilenameFilter; import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -405,9 +404,8 @@ public class DirectoryScanner implements Runnable { diffs.put(bpid, diffRecord); statsRecord.totalBlocks = blockpoolReport.length; - 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 while (m < bl.size() && d < blockpoolReport.length) { 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 78a5cfc9676..cddb7e7e05d 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 7998ff4445a..4b60d679518 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 @@ -1920,17 +1920,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 = datasetLock.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 b119e7855b7..b0b33500e69 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 8f4e29b6e89..d190e95d1e3 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 @@ -1502,7 +1502,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 8fe515f8b75..8cfaa26804b 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 13ffb96f465..91d577f91e0 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; @@ -470,6 +471,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 {