You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/11/22 19:18:46 UTC
[bookkeeper] branch master updated: Remove duplication logic for
'minNumRacksPerWriteQuorum' handling in
RackawareEnsemblePlacementPolicyImpl
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 8c65088 Remove duplication logic for 'minNumRacksPerWriteQuorum' handling in RackawareEnsemblePlacementPolicyImpl
8c65088 is described below
commit 8c65088dcd019fa9fe17d39745ca1400ccf7bbbb
Author: Charan Reddy Guttapalem <re...@gmail.com>
AuthorDate: Thu Nov 22 11:18:42 2018 -0800
Remove duplication logic for 'minNumRacksPerWriteQuorum' handling in RackawareEnsemblePlacementPolicyImpl
Descriptions of the changes in this PR:
With this change
https://github.com/apache/bookkeeper/commit/9ba4c4e0d8be770e03110a958fb8b75a65ae0f59
we introduced 'minNumRacksPerWriteQuorum' config option. To handle that, logic is added in
RackawareEnsemblePlacementPolicyImpl.newEnsembleInternal and RackQuorumCoverageSet. But in
retrospect, this is kind of duplicate logic and changes made in RackawareEnsemblePlacementPolicyImpl.newEnsembleInternal
is not really needed, because anyhow 'apply' of predicate (RRTopologyAwareCoverageEnsemble) is called, which calls
'apply' method on all the concerned RackQuorumCoverageSet.
Reviewers: Sijie Guo <si...@apache.org>
This closes #1810 from reddycharan/revertrackawarechanges
---
.../RackawareEnsemblePlacementPolicyImpl.java | 59 +---------------------
.../TestRackawareEnsemblePlacementPolicy.java | 10 ++--
2 files changed, 8 insertions(+), 61 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
index 1d587d0..9c0feb7 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
@@ -554,9 +554,7 @@ public class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsembleP
}
return addrs;
}
- // pick nodes by racks, to ensure there is at least write quorum number of racks.
- int idx = 0;
- String[] racks = new String[ensembleSize];
+
for (int i = 0; i < ensembleSize; i++) {
String curRack;
if (null == prevNode) {
@@ -566,64 +564,11 @@ public class RackawareEnsemblePlacementPolicyImpl extends TopologyAwareEnsembleP
curRack = localNode.getNetworkLocation();
}
} else {
- StringBuilder sb = new StringBuilder();
- sb.append("~");
-
- if (writeQuorumSize > 1) {
- /*
- * RackAwareEnsemblePlacementPolicy should try to select
- * bookies from atleast
- * minNumRacksPerWriteQuorumForThisEnsemble number of
- * different racks for a write quorum. So in a
- * WriteQuorum, bookies should be from
- * minNumRacksPerWriteQuorumForThisEnsemble number of
- * racks. So we would add racks of
- * (minNumRacksPerWriteQuorumForThisEnsemble-1)
- * neighbours (both sides) to the exclusion list
- * (~curRack).
- */
- for (int j = 1; j < minNumRacksPerWriteQuorumForThisEnsemble; j++) {
- int nextIndex = i + j;
- if (nextIndex >= ensembleSize) {
- nextIndex %= ensembleSize;
- }
- /*
- * if racks[nextIndex] is null, then it means bookie
- * is not yet selected for ensemble at 'nextIndex'
- * index.
- */
- if (racks[nextIndex] != null) {
- if (!((sb.length() == 1) && (sb.charAt(0) == '~'))) {
- sb.append(NetworkTopologyImpl.NODE_SEPARATOR);
- }
- sb.append(racks[nextIndex]);
- }
- }
-
- for (int j = 1; j < minNumRacksPerWriteQuorumForThisEnsemble; j++) {
- int nextIndex = i - j;
- if (nextIndex < 0) {
- nextIndex += ensembleSize;
- }
- /*
- * if racks[nextIndex] is null, then it means bookie
- * is not yet selected for ensemble at 'nextIndex'
- * index.
- */
- if (racks[nextIndex] != null) {
- if (!((sb.length() == 1) && (sb.charAt(0) == '~'))) {
- sb.append(NetworkTopologyImpl.NODE_SEPARATOR);
- }
- sb.append(racks[nextIndex]);
- }
- }
- }
- curRack = sb.toString();
+ curRack = "~" + prevNode.getNetworkLocation();
}
boolean firstBookieInTheEnsemble = (null == prevNode);
prevNode = selectFromNetworkLocation(curRack, excludeNodes, ensemble, ensemble,
!enforceMinNumRacksPerWriteQuorum || firstBookieInTheEnsemble);
- racks[i] = prevNode.getNetworkLocation();
}
List<BookieSocketAddress> bookieList = ensemble.toList();
if (ensembleSize != bookieList.size()) {
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
index dd95b7a..d801044 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
@@ -1200,12 +1200,12 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase {
int acqQuorumSize = 2;
List<BookieSocketAddress> ensemble = repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
null, new HashSet<>());
- int numCovered = getNumCoveredWriteQuorums(ensemble, 2, conf.getMinNumRacksPerWriteQuorum());
+ int numCovered = getNumCoveredWriteQuorums(ensemble, writeQuorumSize, conf.getMinNumRacksPerWriteQuorum());
assertTrue(numCovered >= 1 && numCovered < 3);
ensembleSize = 4;
List<BookieSocketAddress> ensemble2 = repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
null, new HashSet<>());
- numCovered = getNumCoveredWriteQuorums(ensemble2, 2, conf.getMinNumRacksPerWriteQuorum());
+ numCovered = getNumCoveredWriteQuorums(ensemble2, writeQuorumSize, conf.getMinNumRacksPerWriteQuorum());
assertTrue(numCovered >= 1 && numCovered < 3);
} catch (BKNotEnoughBookiesException bnebe) {
fail("Should not get not enough bookies exception even there is only one rack.");
@@ -1319,12 +1319,14 @@ public class TestRackawareEnsemblePlacementPolicy extends TestCase {
int ackQuorumSize = 2;
List<BookieSocketAddress> ensemble1 = repp.newEnsemble(ensembleSize, writeQuorumSize, ackQuorumSize,
null, new HashSet<>());
- assertEquals(ensembleSize, getNumCoveredWriteQuorums(ensemble1, 2, conf.getMinNumRacksPerWriteQuorum()));
+ assertEquals(ensembleSize,
+ getNumCoveredWriteQuorums(ensemble1, writeQuorumSize, conf.getMinNumRacksPerWriteQuorum()));
ensembleSize = 4;
writeQuorumSize = 4;
List<BookieSocketAddress> ensemble2 = repp.newEnsemble(ensembleSize, writeQuorumSize, 2, null,
new HashSet<>());
- assertEquals(ensembleSize, getNumCoveredWriteQuorums(ensemble2, 2, conf.getMinNumRacksPerWriteQuorum()));
+ assertEquals(ensembleSize,
+ getNumCoveredWriteQuorums(ensemble2, writeQuorumSize, conf.getMinNumRacksPerWriteQuorum()));
} catch (BKNotEnoughBookiesException bnebe) {
fail("Should not get not enough bookies exception even there is only one rack.");
}