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 <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 --- .../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); } /**