[bookkeeper] branch branch-4.7 updated: Fix bugs in DefaultEnsemblePlacementPolicy

[sijie]

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

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


The following commit(s) were added to refs/heads/branch-4.7 by this push:
     new 40c6c33  Fix bugs in DefaultEnsemblePlacementPolicy
40c6c33 is described below

commit 40c6c33697ed73fbd3a0b21b365e8aa5becaed20
Author: Charan Reddy Guttapalem <reddychara...@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
    
    (cherry picked from commit 398aa4cdc4b168f76d453fd74eda66182e24b21f)
    Signed-off-by: Sijie Guo <si...@apache.org>
---
 .../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 7602499..a1a67b3 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 36beaa2..66d3d5f 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);
     }
 
     /**