-
Notifications
You must be signed in to change notification settings - Fork 961
Issue 1898: Implement isEnsembleAdheringToPlacementPolicy
in RegionAwareEnsemblePlacementPolicy
#4133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Issue 1898: Implement isEnsembleAdheringToPlacementPolicy
in RegionAwareEnsemblePlacementPolicy
#4133
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -41,6 +41,7 @@ | |||||||||||||||||||||||
import org.apache.bookkeeper.proto.BookieAddressResolver; | ||||||||||||||||||||||||
import org.apache.bookkeeper.stats.StatsLogger; | ||||||||||||||||||||||||
import org.apache.bookkeeper.util.BookKeeperConstants; | ||||||||||||||||||||||||
import org.apache.commons.collections4.CollectionUtils; | ||||||||||||||||||||||||
import org.apache.commons.lang3.tuple.Pair; | ||||||||||||||||||||||||
import org.slf4j.Logger; | ||||||||||||||||||||||||
import org.slf4j.LoggerFactory; | ||||||||||||||||||||||||
|
@@ -644,12 +645,75 @@ public final DistributionSchedule.WriteSet reorderReadLACSequence( | |||||||||||||||||||||||
@Override | ||||||||||||||||||||||||
public PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy(List<BookieId> ensembleList, | ||||||||||||||||||||||||
int writeQuorumSize, int ackQuorumSize) { | ||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* TODO: have to implement actual logic for this method for | ||||||||||||||||||||||||
* RegionAwareEnsemblePlacementPolicy. For now return true value. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* - https://github.com/apache/bookkeeper/issues/1898 | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
if (CollectionUtils.isEmpty(ensembleList)) { | ||||||||||||||||||||||||
return PlacementPolicyAdherence.FAIL; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
int effectiveMinRegionsForDurability = disableDurabilityFeature.isAvailable() ? 1 : minRegionsForDurability; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
int ensembleSize = ensembleList.size(); | ||||||||||||||||||||||||
Map<String, Set<BookieId>> regionsInQuorum = new HashMap<>(); | ||||||||||||||||||||||||
BookieId bookie; | ||||||||||||||||||||||||
for (int i = 0; i < ensembleList.size(); i++) { | ||||||||||||||||||||||||
regionsInQuorum.clear(); | ||||||||||||||||||||||||
for (int j = 0; j < writeQuorumSize; j++) { | ||||||||||||||||||||||||
bookie = ensembleList.get((i + j) % ensembleSize); | ||||||||||||||||||||||||
if (knownBookies.containsKey(bookie)) { | ||||||||||||||||||||||||
String region = getLocalRegion(knownBookies.get(bookie)); | ||||||||||||||||||||||||
if (regionsInQuorum.containsKey(region)) { | ||||||||||||||||||||||||
regionsInQuorum.get(region).add(bookie); | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
Set<BookieId> bookieSet = new HashSet<>(); | ||||||||||||||||||||||||
bookieSet.add(bookie); | ||||||||||||||||||||||||
regionsInQuorum.put(region, bookieSet); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} else if (LOG.isDebugEnabled()) { | ||||||||||||||||||||||||
LOG.debug("bookie {} is not in the list of knownBookies", bookie); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (regionsInQuorum.isEmpty()) { | ||||||||||||||||||||||||
return PlacementPolicyAdherence.FAIL; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (regionsInQuorum.size() < 2) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a question: why judge the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The purpose of doing this here is to align the implement in Lines 356 to 366 in 185fef6
When there is only one available region, fall back to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a little weird. If regionsInQuorum < 2, the result may be If regionsInQuorum = 2, the result is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, a little weird. Should we just remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PlacementPolicyAdherence enum has three values There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||||||||||||||||||||||||
// fall back to use the ensemblePlacementPolicy in specific region | ||||||||||||||||||||||||
String region = regionsInQuorum.keySet().iterator().next(); | ||||||||||||||||||||||||
Set<BookieId> bookieIds = regionsInQuorum.get(region); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
TopologyAwareEnsemblePlacementPolicy policyWithinRegion = perRegionPlacement.get(region); | ||||||||||||||||||||||||
PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy = policyWithinRegion | ||||||||||||||||||||||||
.isEnsembleAdheringToPlacementPolicy(new ArrayList<>(bookieIds), bookieIds.size(), 1); | ||||||||||||||||||||||||
if (isEnsembleAdheringToPlacementPolicy == PlacementPolicyAdherence.FAIL) { | ||||||||||||||||||||||||
if (LOG.isDebugEnabled()) { | ||||||||||||||||||||||||
LOG.debug("For ensemble {}, write set starting at {} are all from one region, " | ||||||||||||||||||||||||
+ "fall back to RackawareEnsemblePlacementPolicy and fail.", ensembleList, i); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return PlacementPolicyAdherence.FAIL; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (effectiveMinRegionsForDurability > 0 && regionsInQuorum.size() < effectiveMinRegionsForDurability) { | ||||||||||||||||||||||||
if (LOG.isDebugEnabled()) { | ||||||||||||||||||||||||
LOG.debug("For ensemble {}, write set starting at {} are from {} regions, " | ||||||||||||||||||||||||
+ "less than effectiveMinRegionsForDurability: {}.", | ||||||||||||||||||||||||
ensembleList, i, regionsInQuorum.size(), effectiveMinRegionsForDurability); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return PlacementPolicyAdherence.FAIL; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (regionsInQuorum.size() < writeQuorumSize) { | ||||||||||||||||||||||||
dragonls marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
// each writeQuorum should be different regions | ||||||||||||||||||||||||
if (LOG.isDebugEnabled()) { | ||||||||||||||||||||||||
LOG.debug("For ensemble: {}, write set starting at {} are from {} regions, " | ||||||||||||||||||||||||
+ "less than writeQuorumSize {}.", | ||||||||||||||||||||||||
ensembleList, i, regionsInQuorum.size(), writeQuorumSize); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return PlacementPolicyAdherence.FAIL; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return PlacementPolicyAdherence.MEETS_STRICT; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} |
Uh oh!
There was an error while loading. Please reload this page.