From 615a2a42cf428dc687809508ef36a1fc5cba0099 Mon Sep 17 00:00:00 2001 From: GuoPhilipse <46367746+GuoPhilipse@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:01:32 +0800 Subject: [PATCH] HDFS-17220. Fix same available space policy in AvailableSpaceVolumeChoosingPolicy (#6174) Reviewed-by: He Xiaoqiao Reviewed-by: zhangshuyan Signed-off-by: Tao Li --- .../AvailableSpaceVolumeChoosingPolicy.java | 2 +- ...estAvailableSpaceVolumeChoosingPolicy.java | 76 ++++++++++++++++--- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/AvailableSpaceVolumeChoosingPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/AvailableSpaceVolumeChoosingPolicy.java index 5d12fa72bb..19740ca060 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/AvailableSpaceVolumeChoosingPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/AvailableSpaceVolumeChoosingPolicy.java @@ -209,7 +209,7 @@ public boolean areAllVolumesWithinFreeSpaceThreshold() { leastAvailable = Math.min(leastAvailable, volume.getAvailable()); mostAvailable = Math.max(mostAvailable, volume.getAvailable()); } - return (mostAvailable - leastAvailable) < balancedSpaceThreshold; + return (mostAvailable - leastAvailable) <= balancedSpaceThreshold; } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/TestAvailableSpaceVolumeChoosingPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/TestAvailableSpaceVolumeChoosingPolicy.java index 24a43e7a91..2243398c8a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/TestAvailableSpaceVolumeChoosingPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/TestAvailableSpaceVolumeChoosingPolicy.java @@ -35,16 +35,18 @@ public class TestAvailableSpaceVolumeChoosingPolicy { private static final int RANDOMIZED_ITERATIONS = 10000; + private static final int BALANCED_SPACE_THRESHOLD = 1024 * 1024; // 1MB private static final float RANDOMIZED_ERROR_PERCENT = 0.05f; private static final long RANDOMIZED_ALLOWED_ERROR = (long) (RANDOMIZED_ERROR_PERCENT * RANDOMIZED_ITERATIONS); private static void initPolicy(VolumeChoosingPolicy policy, + long balanceSpaceThreshold, float preferencePercent) { Configuration conf = new Configuration(); // Set the threshold to consider volumes imbalanced to 1MB conf.setLong( DFS_DATANODE_AVAILABLE_SPACE_VOLUME_CHOOSING_POLICY_BALANCED_SPACE_THRESHOLD_KEY, - 1024 * 1024); // 1MB + balanceSpaceThreshold); conf.setFloat( DFS_DATANODE_AVAILABLE_SPACE_VOLUME_CHOOSING_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY, preferencePercent); @@ -58,7 +60,7 @@ public void testRR() throws Exception { @SuppressWarnings("unchecked") final AvailableSpaceVolumeChoosingPolicy policy = ReflectionUtils.newInstance(AvailableSpaceVolumeChoosingPolicy.class, null); - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); TestRoundRobinVolumeChoosingPolicy.testRR(policy); } @@ -68,7 +70,7 @@ public void testRR() throws Exception { public void testRRPolicyExceptionMessage() throws Exception { final AvailableSpaceVolumeChoosingPolicy policy = new AvailableSpaceVolumeChoosingPolicy(); - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); TestRoundRobinVolumeChoosingPolicy.testRRPolicyExceptionMessage(policy); } @@ -77,7 +79,7 @@ public void testTwoUnbalancedVolumes() throws Exception { @SuppressWarnings("unchecked") final AvailableSpaceVolumeChoosingPolicy policy = ReflectionUtils.newInstance(AvailableSpaceVolumeChoosingPolicy.class, null); - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); List volumes = new ArrayList(); @@ -120,7 +122,7 @@ public void testThreeUnbalancedVolumes() throws Exception { // We should alternate assigning between the two volumes with a lot of free // space. - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); Assert.assertEquals(volumes.get(1), policy.chooseVolume(volumes, 100, null)); Assert.assertEquals(volumes.get(2), policy.chooseVolume(volumes, 100, @@ -131,7 +133,7 @@ public void testThreeUnbalancedVolumes() throws Exception { null)); // All writes should be assigned to the volume with the least free space. - initPolicy(policy, 0.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 0.0f); Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, null)); Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, @@ -141,7 +143,57 @@ public void testThreeUnbalancedVolumes() throws Exception { Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, null)); } - + + + @Test(timeout=60000) + public void testSameAvailableVolumeSpace() throws Exception { + @SuppressWarnings("unchecked") + final AvailableSpaceVolumeChoosingPolicy policy = + ReflectionUtils.newInstance(AvailableSpaceVolumeChoosingPolicy.class, null); + + List volumes = new ArrayList(); + + // first volume with 1MB free space + volumes.add(Mockito.mock(FsVolumeSpi.class)); + Mockito.when(volumes.get(0).getAvailable()).thenReturn(1024L * 1024L); + + // Second volume with 1MB free space + volumes.add(Mockito.mock(FsVolumeSpi.class)); + Mockito.when(volumes.get(1).getAvailable()).thenReturn(1024L * 1024L); + + // Third volume with 1MB free space + volumes.add(Mockito.mock(FsVolumeSpi.class)); + Mockito.when(volumes.get(2).getAvailable()).thenReturn(1024L * 1024L); + + // Fourth volume with 1MB free space + volumes.add(Mockito.mock(FsVolumeSpi.class)); + Mockito.when(volumes.get(3).getAvailable()).thenReturn(1024L * 1024L); + + // We should alternate assigning between all the above volumes + // for they have the same available space + initPolicy(policy, 0, 1.0f); + Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(1), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(2), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(3), policy.chooseVolume(volumes, 100, + null)); + + // We should alternate assigning between all the above volumes + // for they have the same available space + initPolicy(policy, 0, 0.0f); + Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(1), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(2), policy.chooseVolume(volumes, 100, + null)); + Assert.assertEquals(volumes.get(3), policy.chooseVolume(volumes, 100, + null)); + } + @Test(timeout=60000) public void testFourUnbalancedVolumes() throws Exception { @SuppressWarnings("unchecked") @@ -169,7 +221,7 @@ public void testFourUnbalancedVolumes() throws Exception { // We should alternate assigning between the two volumes with a lot of free // space. - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); Assert.assertEquals(volumes.get(2), policy.chooseVolume(volumes, 100, null)); Assert.assertEquals(volumes.get(3), policy.chooseVolume(volumes, 100, @@ -181,7 +233,7 @@ public void testFourUnbalancedVolumes() throws Exception { // We should alternate assigning between the two volumes with less free // space. - initPolicy(policy, 0.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 0.0f); Assert.assertEquals(volumes.get(0), policy.chooseVolume(volumes, 100, null)); Assert.assertEquals(volumes.get(1), policy.chooseVolume(volumes, 100, @@ -213,7 +265,7 @@ public void testNotEnoughSpaceOnSelectedVolume() throws Exception { // However, if the volume with the least free space doesn't have enough // space to accept the replica size, and another volume does have enough // free space, that should be chosen instead. - initPolicy(policy, 0.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 0.0f); Assert.assertEquals(volumes.get(1), policy.chooseVolume(volumes, 1024L * 1024L * 2, null)); } @@ -223,7 +275,7 @@ public void testAvailableSpaceChanges() throws Exception { @SuppressWarnings("unchecked") final AvailableSpaceVolumeChoosingPolicy policy = ReflectionUtils.newInstance(AvailableSpaceVolumeChoosingPolicy.class, null); - initPolicy(policy, 1.0f); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, 1.0f); List volumes = new ArrayList(); @@ -292,7 +344,7 @@ public void doRandomizedTest(float preferencePercent, int lowSpaceVolumes, volumes.add(volume); } - initPolicy(policy, preferencePercent); + initPolicy(policy, BALANCED_SPACE_THRESHOLD, preferencePercent); long lowAvailableSpaceVolumeSelected = 0; long highAvailableSpaceVolumeSelected = 0; for (int i = 0; i < RANDOMIZED_ITERATIONS; i++) {