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/17 04:46:04 UTC
[hadoop] branch branch-3.2 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 branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.2 by this push:
new aa582cc HDFS-15574. Remove unnecessary sort of block list in DirectoryScanner. Contributed by Stephen O'Donnell.
aa582cc is described below
commit aa582ccc2a2d7fa08cfa1a04d4cfa28c40183f14
Author: hemanthboyina <he...@apache.org>
AuthorDate: Thu Sep 17 10:15:18 2020 +0530
HDFS-15574. Remove unnecessary sort of block list in DirectoryScanner. Contributed by Stephen O'Donnell.
---
.../hdfs/server/datanode/DirectoryScanner.java | 6 ++--
.../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, 50 insertions(+), 14 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 99584d9..aede03e 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<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
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 78a5cfc..cddb7e7 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 7998ff4..4b60d67 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<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 = datasetLock.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 b119e78..b0b3350 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 8f4e29b..d190e95 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<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 8fe515f..8cfaa26 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 13ffb96..91d577f 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<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