[bookkeeper] branch master updated: Fix bugs in DefaultEnsemblePlacementPolicy

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 398aa4c  Fix bugs in DefaultEnsemblePlacementPolicy
398aa4c is described below

commit 398aa4cdc4b168f76d453fd74eda66182e24b21f
Author: Charan Reddy Guttapalem <re...@gmail.com>
AuthorDate: Sun Nov 11 13:30:15 2018 -0800

    Fix bugs in DefaultEnsemblePlacementPolicy
    
    
    Descriptions of the changes in this PR:
    
    - bookieInfoMap is not initialized and newEnsemble will throws BKNotEnoughBookiesException if
    diskWeightBasedPlacement is enabled
    - add test coverage for DefaultEnsemblePlacementPolicy with diskWeightBasedPlacement enabled
    
    
    
    Reviewers: Sijie Guo <si...@apache.org>, Andrey Yegorov <None>
    
    This closes #1788 from reddycharan/defaultplacementfix
---
 .../bookkeeper/client/DefaultEnsemblePlacementPolicy.java  |  5 +++++
 .../client/GenericEnsemblePlacementPolicyTest.java         | 14 +++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java
index 28efe66..917d18f 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultEnsemblePlacementPolicy.java
@@ -22,6 +22,7 @@ import io.netty.util.HashedWheelTimer;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -58,6 +59,7 @@ public class DefaultEnsemblePlacementPolicy implements EnsemblePlacementPolicy {
     private final ReentrantReadWriteLock rwLock;
 
     DefaultEnsemblePlacementPolicy() {
+        bookieInfoMap = new HashMap<BookieSocketAddress, WeightedObject>();
         rwLock = new ReentrantReadWriteLock();
     }
 
@@ -92,6 +94,9 @@ public class DefaultEnsemblePlacementPolicy implements EnsemblePlacementPolicy {
                     }
                     newBookies.add(b);
                     --ensembleSize;
+                    if (ensembleSize == 0) {
+                        return newBookies;
+                    }
                 }
             } finally {
                 rwLock.readLock().unlock();
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/GenericEnsemblePlacementPolicyTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/GenericEnsemblePlacementPolicyTest.java
index bf369cf..8fbb009 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/GenericEnsemblePlacementPolicyTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/GenericEnsemblePlacementPolicyTest.java
@@ -24,6 +24,8 @@ import static org.junit.Assert.fail;
 
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -33,10 +35,14 @@ import org.apache.bookkeeper.net.BookieSocketAddress;
 import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
 
 /**
  * Testing a generic ensemble placement policy.
  */
+@RunWith(Parameterized.class)
 public class GenericEnsemblePlacementPolicyTest extends BookKeeperClusterTestCase {
 
     private BookKeeper.DigestType digestType = BookKeeper.DigestType.CRC32;
@@ -46,9 +52,15 @@ public class GenericEnsemblePlacementPolicyTest extends BookKeeperClusterTestCas
     private static List<Map<String, byte[]>> customMetadataOnNewEnsembleStack = new ArrayList<>();
     private static List<Map<String, byte[]>> customMetadataOnReplaceBookieStack = new ArrayList<>();
 
-    public GenericEnsemblePlacementPolicyTest() {
+    @Parameters
+    public static Collection<Object[]> getDiskWeightBasedPlacementEnabled() {
+        return Arrays.asList(new Object[][] { { false }, { true } });
+    }
+
+    public GenericEnsemblePlacementPolicyTest(boolean diskWeightBasedPlacementEnabled) {
         super(0);
         baseClientConf.setEnsemblePlacementPolicy(CustomEnsemblePlacementPolicy.class);
+        baseClientConf.setDiskWeightBasedPlacementEnabled(diskWeightBasedPlacementEnabled);
     }
 
     /**