You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by he...@apache.org on 2020/09/15 04:41:37 UTC
[hadoop] branch trunk updated: HDFS-15574. Remove unnecessary sort
of block list in DirectoryScanner. Contributed by Stephen O'Donnell.
This is an automated email from the ASF dual-hosted git repository.
hemanthboyina pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new f4ed9f3 HDFS-15574. Remove unnecessary sort of block list in DirectoryScanner. Contributed by Stephen O'Donnell.
f4ed9f3 is described below
commit f4ed9f3f911d727e21a9327bec1ea4ba2cfd7966
Author: hemanthboyina <he...@apache.org>
AuthorDate: Tue Sep 15 10:10:21 2020 +0530
HDFS-15574. Remove unnecessary sort of block list in DirectoryScanner. Contributed by Stephen O'Donnell.
---
.../hdfs/server/datanode/DirectoryScanner.java | 3 +-
.../server/datanode/fsdataset/FsDatasetSpi.java | 7 +++--
.../datanode/fsdataset/impl/FsDatasetImpl.java | 7 +++--
.../org/apache/hadoop/hdfs/TestCrcCorruption.java | 2 +-
.../hadoop/hdfs/TestReconstructStripedFile.java | 2 +-
.../hdfs/server/datanode/SimulatedFSDataset.java | 2 +-
.../datanode/extdataset/ExternalDatasetImpl.java | 2 +-
.../datanode/fsdataset/impl/TestFsDatasetImpl.java | 36 ++++++++++++++++++++++
8 files changed, 49 insertions(+), 12 deletions(-)
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 b2e521c..39b8a3f 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<ScanInfo> diffRecord = new ArrayList<>();
statsRecord.totalBlocks = blockpoolReport.size();
- final List<ReplicaInfo> bl = dataset.getFinalizedBlocks(bpid);
- Collections.sort(bl); // Sort based on blockId
+ final List<ReplicaInfo> 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 177c62e..89ad510 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<V extends FsVolumeSpi> 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.
* <p>
* Callers of this function should call
* {@link FsDatasetSpi#acquireDatasetLock} to avoid blocks' status being
* changed during list iteration.
* </p>
* @return a list of references to the finalized blocks for the given block
- * pool.
+ * pool. The list is sorted by blockID.
*/
- List<ReplicaInfo> getFinalizedBlocks(String bpid);
+ List<ReplicaInfo> 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 de898e9..0ae08f6 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<FsVolumeImpl> {
}
/**
- * 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.
* <p>
* Callers of this function should call
* {@link FsDatasetSpi#acquireDatasetLock()} to avoid blocks' status being
* changed during list iteration.
* </p>
* @return a list of references to the finalized blocks for the given block
- * pool.
+ * pool. The list is sorted by blockID.
*/
@Override
- public List<ReplicaInfo> getFinalizedBlocks(String bpid) {
+ public List<ReplicaInfo> getSortedFinalizedBlocks(String bpid) {
try (AutoCloseableLock lock = datasetReadLock.acquire()) {
final List<ReplicaInfo> finalized = new ArrayList<ReplicaInfo>(
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 917f0db..df6a7dc 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<ReplicaInfo> 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 6156c3d..65cf385 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 8069b64..d9d7630 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<FsVolumeSpi> {
}
@Override
- public List<ReplicaInfo> getFinalizedBlocks(String bpid) {
+ public List<ReplicaInfo> 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 caaa89c..13b740e 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<ExternalVolumeImpl> {
}
@Override
- public List<ReplicaInfo> getFinalizedBlocks(String bpid) {
+ public List<ReplicaInfo> 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 626ccee..0e61220 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<ReplicaInfo> replicaList = ds.getSortedFinalizedBlocks(BLOCKPOOL);
+ for (int i=0; i<replicaList.size() - 1; i++) {
+ if (replicaList.get(i).compareTo(replicaList.get(i+1)) > 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 {
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org