sijie commented on a change in pull request #8961:
URL: https://github.com/apache/pulsar/pull/8961#discussion_r549594635
##########
File path: pulsar-zookeeper-utils/pom.xml
##########
@@ -106,6 +106,13 @@
<artifactId>simpleclient_caffeine</artifactId>
</dependency>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>managed-ledger</artifactId>
Review comment:
Can we avoid introducing manager-ledger as the dependency of
`pulsar-zookeeper-utils`?
##########
File path:
pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java
##########
@@ -156,34 +177,79 @@ private ZooKeeperCache getAndSetZkCache(Configuration
conf) {
bookieToReplace, excludeBookies);
}
- private Set<BookieId> getBlacklistedBookies(int ensembleSize) {
- Set<BookieId> blacklistedBookies = new HashSet<BookieId>();
+ private static Optional<EnsemblePlacementPolicyConfig>
getEnsemblePlacementPolicyConfig(
+ Map<String, byte[]> customMetadata) {
+
+ byte[] ensemblePlacementPolicyConfigData = customMetadata.get(
+ EnsemblePlacementPolicyConfig.ENSEMBLE_PLACEMENT_POLICY_CONFIG);
+ if (ensemblePlacementPolicyConfigData != null) {
+ try {
+ return
Optional.ofNullable(EnsemblePlacementPolicyConfig.decode(ensemblePlacementPolicyConfigData));
+ } catch (JsonUtil.ParseJsonException e) {
+ LOG.error("Failed to parse the ensemble placement policy
config from the custom metadata", e);
+ return Optional.empty();
+ }
+ }
+ return Optional.empty();
+ }
+
+ private Map<String, List<String>> getDefaultIsolationGroups() {
+ Map<String, List<String>> isolationGroup = new HashMap<>();
+ isolationGroup.put(ISOLATION_BOOKIE_GROUPS, primaryIsolationGroups);
+ isolationGroup.put(SECONDARY_ISOLATION_BOOKIE_GROUPS,
secondaryIsolationGroups);
+ return isolationGroup;
+ }
+
+ private static Map<String, List<String>>
getIsolationGroup(EnsemblePlacementPolicyConfig ensemblePlacementPolicyConfig) {
Review comment:
I think you can use `Pair<Set<String>, Set<String>>` instead of a
HashMap. Because you only have two sets of groups, one is the primary group and
the other one is a secondary group. A HashHamp introduces a lot of overheads.
##########
File path: pulsar-zookeeper-utils/pom.xml
##########
@@ -106,6 +106,13 @@
<artifactId>simpleclient_caffeine</artifactId>
</dependency>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>managed-ledger</artifactId>
Review comment:
I'd suggest moving EnsemblePlacementPolicyConfig to `pulsar-common`. So
it can be used in both `pulsar-zookeeper-utils` and `managed-ledger`.
##########
File path:
pulsar-zookeeper-utils/src/main/java/org/apache/pulsar/zookeeper/ZkIsolatedBookieEnsemblePlacementPolicy.java
##########
@@ -156,34 +177,79 @@ private ZooKeeperCache getAndSetZkCache(Configuration
conf) {
bookieToReplace, excludeBookies);
}
- private Set<BookieId> getBlacklistedBookies(int ensembleSize) {
- Set<BookieId> blacklistedBookies = new HashSet<BookieId>();
+ private static Optional<EnsemblePlacementPolicyConfig>
getEnsemblePlacementPolicyConfig(
+ Map<String, byte[]> customMetadata) {
+
+ byte[] ensemblePlacementPolicyConfigData = customMetadata.get(
+ EnsemblePlacementPolicyConfig.ENSEMBLE_PLACEMENT_POLICY_CONFIG);
+ if (ensemblePlacementPolicyConfigData != null) {
+ try {
+ return
Optional.ofNullable(EnsemblePlacementPolicyConfig.decode(ensemblePlacementPolicyConfigData));
+ } catch (JsonUtil.ParseJsonException e) {
+ LOG.error("Failed to parse the ensemble placement policy
config from the custom metadata", e);
+ return Optional.empty();
+ }
+ }
+ return Optional.empty();
+ }
+
+ private Map<String, List<String>> getDefaultIsolationGroups() {
+ Map<String, List<String>> isolationGroup = new HashMap<>();
Review comment:
Can you create the defaultIsolationGroups when creating the ensemble
placement policy? Because the default isolation groups don't change after
creations, correct?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
