Skip to content

Commit

Permalink
Fixes the recovery not respect to the isolation group settings (apach…
Browse files Browse the repository at this point in the history
…e#8961)

---

*Motivation*

When users configure to use ZkIsolatedBookieEnsemblePlacementPolicy,
it is hard to configure AutoRecovery to respect to the isolation group
settings. Because we don't store the isolation group information as
part of ledger metadata, the framework doesn't have any information
to use for choosing the bookies.

*Modifications*

- Change the ZkIsolatedBookieEnsemblePlacementPolicy to use the policy
passed from the custom metadata
  • Loading branch information
zymap authored Dec 30, 2020
1 parent 78072fe commit bddd030
Show file tree
Hide file tree
Showing 10 changed files with 298 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
package org.apache.bookkeeper.mledger.impl;

import com.google.common.collect.ImmutableMap;
import org.apache.bookkeeper.client.EnsemblePlacementPolicy;
import org.apache.bookkeeper.common.util.JsonUtil.ParseJsonException;
import org.apache.pulsar.common.policies.data.EnsemblePlacementPolicyConfig;

import java.nio.charset.StandardCharsets;
import java.util.Map;

Expand Down Expand Up @@ -99,6 +103,24 @@ public static Map<String, byte[]> buildMetadataForSchema(String schemaId) {
);
}

/**
* Build additional metadata for the placement policy config.
*
* @param className
* the ensemble placement policy classname
* @param properties
* the ensemble placement policy properties
* @return
* the additional metadata
* @throws ParseJsonException
* placement policy configuration encode error
*/
static Map<String, byte[]> buildMetadataForPlacementPolicyConfig(
Class<? extends EnsemblePlacementPolicy> className, Map<String, Object> properties) throws ParseJsonException {
EnsemblePlacementPolicyConfig config = new EnsemblePlacementPolicyConfig(className, properties);
return ImmutableMap.of(EnsemblePlacementPolicyConfig.ENSEMBLE_PLACEMENT_POLICY_CONFIG, config.encode());
}

private LedgerMetadataUtils() {}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static org.apache.bookkeeper.mledger.ManagedLedgerException.getManagedLedgerException;

import com.google.common.base.Objects;
import com.google.common.base.Predicates;
import com.google.common.collect.Maps;

Expand All @@ -45,7 +44,6 @@

import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.BookKeeper;
import org.apache.bookkeeper.client.EnsemblePlacementPolicy;
import org.apache.bookkeeper.common.util.OrderedExecutor;
import org.apache.bookkeeper.common.util.OrderedScheduler;
import org.apache.bookkeeper.conf.ClientConfiguration;
Expand Down Expand Up @@ -81,6 +79,7 @@
import org.apache.bookkeeper.stats.StatsLogger;
import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
import org.apache.pulsar.common.util.DateFormatter;
import org.apache.pulsar.common.policies.data.EnsemblePlacementPolicyConfig;
import org.apache.pulsar.metadata.api.MetadataStore;
import org.apache.pulsar.metadata.api.Stat;
import org.apache.pulsar.metadata.impl.zookeeper.ZKMetadataStore;
Expand Down Expand Up @@ -871,41 +870,5 @@ default BookKeeper get() {
BookKeeper get(EnsemblePlacementPolicyConfig ensemblePlacementPolicyMetadata);
}

public static class EnsemblePlacementPolicyConfig {
private final Class<? extends EnsemblePlacementPolicy> policyClass;
private final Map<String, Object> properties;

public EnsemblePlacementPolicyConfig(Class<? extends EnsemblePlacementPolicy> policyClass,
Map<String, Object> properties) {
super();
this.policyClass = policyClass;
this.properties = properties;
}

public Class<? extends EnsemblePlacementPolicy> getPolicyClass() {
return policyClass;
}

public Map<String, Object> getProperties() {
return properties;
}

@Override
public int hashCode() {
return Objects.hashCode(policyClass != null ? policyClass.getName() : "", properties);
}

@Override
public boolean equals(Object obj) {
if (obj instanceof EnsemblePlacementPolicyConfig) {
EnsemblePlacementPolicyConfig other = (EnsemblePlacementPolicyConfig) obj;
return Objects.equal(this.policyClass == null ? null : this.policyClass.getName(),
other.policyClass == null ? null : other.policyClass.getName())
&& Objects.equal(this.properties, other.properties);
}
return false;
}
}

private static final Logger log = LoggerFactory.getLogger(ManagedLedgerFactoryImpl.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import io.netty.util.ReferenceCountUtil;
import org.apache.bookkeeper.client.AsyncCallback;
import org.apache.bookkeeper.client.AsyncCallback.CreateCallback;
Expand All @@ -76,6 +75,7 @@
import org.apache.bookkeeper.client.LedgerHandle;
import org.apache.bookkeeper.client.api.ReadHandle;
import org.apache.bookkeeper.common.util.Backoff;
import org.apache.bookkeeper.common.util.JsonUtil;
import org.apache.bookkeeper.common.util.OrderedExecutor;
import org.apache.bookkeeper.common.util.OrderedScheduler;
import org.apache.bookkeeper.common.util.Retries;
Expand Down Expand Up @@ -256,6 +256,13 @@ public enum PositionBound {
*/
final ConcurrentLinkedQueue<OpAddEntry> pendingAddEntries = new ConcurrentLinkedQueue<>();

/**
* This variable is used for testing the tests
* {@link ManagedLedgerTest#testManagedLedgerWithPlacementPolicyInCustomMetadata()}
*/
@VisibleForTesting
Map<String, byte[]> createdLedgerCustomMetadata;

// //////////////////////////////////////////////////////////////////////

public ManagedLedgerImpl(ManagedLedgerFactoryImpl factory, BookKeeper bookKeeper, MetaStore store,
Expand Down Expand Up @@ -3383,6 +3390,20 @@ protected void asyncCreateLedger(BookKeeper bookKeeper, ManagedLedgerConfig conf
Map<String, byte[]> finalMetadata = new HashMap<>();
finalMetadata.putAll(ledgerMetadata);
finalMetadata.putAll(metadata);
if (config.getBookKeeperEnsemblePlacementPolicyClassName() != null
&& config.getBookKeeperEnsemblePlacementPolicyProperties() != null) {
try {
finalMetadata.putAll(LedgerMetadataUtils.buildMetadataForPlacementPolicyConfig(
config.getBookKeeperEnsemblePlacementPolicyClassName(),
config.getBookKeeperEnsemblePlacementPolicyProperties()
));
} catch (JsonUtil.ParseJsonException e) {
log.error("[{}] Serialize the placement configuration failed", name, e);
cb.createComplete(Code.UnexpectedConditionException, null, ledgerCreated);
return;
}
}
createdLedgerCustomMetadata = finalMetadata;
log.info("[{}] Creating ledger, metadata: {} - metadata ops timeout : {} seconds",
name, finalMetadata, config.getMetadataOperationsTimeoutSeconds());
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.bookkeeper.mledger.impl;

import org.apache.pulsar.common.policies.data.EnsemblePlacementPolicyConfig;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down Expand Up @@ -69,6 +70,7 @@
import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.BookKeeper;
import org.apache.bookkeeper.client.BookKeeper.DigestType;
import org.apache.bookkeeper.client.EnsemblePlacementPolicy;
import org.apache.bookkeeper.client.LedgerHandle;
import org.apache.bookkeeper.client.PulsarMockBookKeeper;
import org.apache.bookkeeper.client.PulsarMockLedgerHandle;
Expand Down Expand Up @@ -2791,6 +2793,23 @@ public void deleteWithoutOpen() throws Exception {
}
}

private abstract class MockedPlacementPolicy implements EnsemblePlacementPolicy{}

@Test(timeOut = 10000)
public void testManagedLedgerWithPlacementPolicyInCustomMetadata() throws Exception {
ManagedLedgerConfig managedLedgerConfig = new ManagedLedgerConfig();
managedLedgerConfig.setBookKeeperEnsemblePlacementPolicyClassName(MockedPlacementPolicy.class);
managedLedgerConfig.setBookKeeperEnsemblePlacementPolicyProperties(Collections.singletonMap("key", "value"));
ManagedLedgerImpl ledger = (ManagedLedgerImpl) factory.open("my_test_ledger", managedLedgerConfig);
assertFalse(ledger.createdLedgerCustomMetadata.isEmpty());
byte[] configData = ledger.createdLedgerCustomMetadata.get(EnsemblePlacementPolicyConfig.ENSEMBLE_PLACEMENT_POLICY_CONFIG);
EnsemblePlacementPolicyConfig config = EnsemblePlacementPolicyConfig.decode(configData);
assertEquals(config.getPolicyClass().getName(), MockedPlacementPolicy.class.getName());
assertEquals(config.getProperties().size(), 1);
assertTrue(config.getProperties().containsKey("key"));
assertEquals(config.getProperties().get("key"), "value");
}

private void setFieldValue(Class clazz, Object classObj, String fieldName, Object fieldValue) throws Exception {
Field field = clazz.getDeclaredField(fieldName);
field.setAccessible(true);
Expand All @@ -2806,4 +2825,4 @@ public static void retryStrategically(Predicate<Void> predicate, int retryCount,
Thread.sleep(intSleepTimeInMillis + (intSleepTimeInMillis * i));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@
import org.apache.bookkeeper.mledger.ManagedLedgerFactoryConfig;
import org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl;
import org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.BookkeeperFactoryForCustomEnsemblePlacementPolicy;
import org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.EnsemblePlacementPolicyConfig;
import org.apache.bookkeeper.stats.NullStatsProvider;
import org.apache.bookkeeper.stats.StatsLogger;
import org.apache.bookkeeper.stats.StatsProvider;
import org.apache.commons.configuration.Configuration;
import org.apache.pulsar.broker.stats.prometheus.metrics.PrometheusMetricsProvider;
import org.apache.pulsar.common.policies.data.EnsemblePlacementPolicyConfig;
import org.apache.zookeeper.ZooKeeper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.apache.bookkeeper.client.api.LedgerMetadata;
import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.meta.LedgerManager;
import org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.EnsemblePlacementPolicyConfig;
import org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl;
import org.apache.bookkeeper.mledger.proto.MLDataFormats.ManagedLedgerInfo.LedgerInfo;
import org.apache.bookkeeper.net.BookieId;
Expand All @@ -63,6 +62,7 @@
import org.apache.pulsar.common.policies.data.BookieInfo;
import org.apache.pulsar.common.policies.data.BookiesRackConfiguration;
import org.apache.pulsar.common.policies.data.ClusterData;
import org.apache.pulsar.common.policies.data.EnsemblePlacementPolicyConfig;
import org.apache.pulsar.common.policies.data.TenantInfo;
import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.apache.pulsar.zookeeper.LocalBookkeeperEnsemble;
Expand Down
5 changes: 5 additions & 0 deletions pulsar-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@
</exclusions>
</dependency>

<dependency>
<groupId>org.apache.bookkeeper</groupId>
<artifactId>bookkeeper-common</artifactId>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>aircompressor</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.pulsar.common.policies.data;

import com.google.common.base.Objects;
import org.apache.bookkeeper.common.util.JsonUtil;

import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Map;

public class EnsemblePlacementPolicyConfig {
public static final String ENSEMBLE_PLACEMENT_POLICY_CONFIG = "EnsemblePlacementPolicyConfig";
private final Class policyClass;
private final Map<String, Object> properties;

// Add a default constructor for decode data from bytes to construct this.
private EnsemblePlacementPolicyConfig() {
this.policyClass = null;
this.properties = Collections.emptyMap();
}

public EnsemblePlacementPolicyConfig(Class policyClass, Map<String, Object> properties) {
super();
this.policyClass = policyClass;
this.properties = properties;
}

public Class getPolicyClass() {
return policyClass;
}

public Map<String, Object> getProperties() {
return properties;
}

@Override
public int hashCode() {
return Objects.hashCode(policyClass != null ? policyClass.getName() : "", properties);
}

@Override
public boolean equals(Object obj) {
if (obj instanceof EnsemblePlacementPolicyConfig) {
EnsemblePlacementPolicyConfig other = (EnsemblePlacementPolicyConfig) obj;
return Objects.equal(this.policyClass == null ? null : this.policyClass.getName(),
other.policyClass == null ? null : other.policyClass.getName())
&& Objects.equal(this.properties, other.properties);
}
return false;
}

public byte[] encode() throws JsonUtil.ParseJsonException {
return JsonUtil.toJson(this).getBytes(StandardCharsets.UTF_8);
}

public static EnsemblePlacementPolicyConfig decode(byte[] data) throws JsonUtil.ParseJsonException {
return JsonUtil.fromJson(new String(data, StandardCharsets.UTF_8), EnsemblePlacementPolicyConfig.class);
}
}
Loading

0 comments on commit bddd030

Please sign in to comment.