ivankelly closed pull request #1217: Shade bookkeeper in
distributedlog-core-shaded and allow using shaded bookkeeper client to resolve
ledger manager factory class
URL: https://github.com/apache/bookkeeper/pull/1217
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java
index 9c90c333e..72f4d0f21 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java
@@ -73,6 +73,8 @@
// Ledger Manager
protected static final String LEDGER_MANAGER_TYPE = "ledgerManagerType";
protected static final String LEDGER_MANAGER_FACTORY_CLASS =
"ledgerManagerFactoryClass";
+ protected static final String ALLOW_SHADED_LEDGER_MANAGER_FACTORY_CLASS =
"allowShadedLedgerManagerFactoryClass";
+ protected static final String SHADED_LEDGER_MANAGER_FACTORY_CLASS_PREFIX =
"shadedLedgerManagerFactoryClassPrefix";
protected static final String METADATA_SERVICE_URI = "metadataServiceUri";
protected static final String ZK_LEDGERS_ROOT_PATH = "zkLedgersRootPath";
protected static final String ZK_REQUEST_RATE_LIMIT = "zkRequestRateLimit";
@@ -310,6 +312,60 @@ public String getLedgerManagerType() {
return getString(LEDGER_MANAGER_TYPE);
}
+ /**
+ * Set the flag to allow using shaded ledger manager factory class for
+ * instantiating a ledger manager factory.
+ *
+ * @param allowed
+ * the flag to allow/disallow using shaded ledger manager factory
class
+ * @return configuration instance.
+ */
+ public T setAllowShadedLedgerManagerFactoryClass(boolean allowed) {
+ setProperty(ALLOW_SHADED_LEDGER_MANAGER_FACTORY_CLASS, allowed);
+ return getThis();
+ }
+
+ /**
+ * Is shaded ledger manager factory class name allowed to be used for
+ * instantiating ledger manager factory.
+ *
+ * @return ledger manager factory class name.
+ */
+ public boolean isShadedLedgerManagerFactoryClassAllowed() {
+ return getBoolean(ALLOW_SHADED_LEDGER_MANAGER_FACTORY_CLASS, false);
+ }
+
+ /**
+ * Set the class prefix of the shaded ledger manager factory class for
+ * instantiating a ledger manager factory.
+ *
+ * <p>This setting only takes effects when {@link
#isShadedLedgerManagerFactoryClassAllowed()}
+ * returns true.
+ *
+ * @param classPrefix
+ * the class prefix of shaded ledger manager factory class
+ * @return configuration instance.
+ * @see #setAllowLedgerManagerFactoryClass(boolean)
+ */
+ public T setShadedLedgerManagerFactoryClassPrefix(String classPrefix) {
+ setProperty(SHADED_LEDGER_MANAGER_FACTORY_CLASS_PREFIX, classPrefix);
+ return getThis();
+ }
+
+ /**
+ * Get the class prefix of the shaded ledger manager factory class name
allowed to be used for
+ * instantiating ledger manager factory.
+ *
+ * <p>This setting only takes effects when {@link
#isShadedLedgerManagerFactoryClassAllowed()}
+ * returns true
+ *
+ * @return ledger manager factory class name.
+ * @see #isShadedLedgerManagerFactoryClassAllowed()
+ */
+ public String getShadedLedgerManagerFactoryClassPrefix() {
+ return getString(SHADED_LEDGER_MANAGER_FACTORY_CLASS_PREFIX,
"dlshade.");
+ }
+
/**
* Set Ledger Manager Factory Class Name.
*
@@ -320,6 +376,15 @@ public void setLedgerManagerFactoryClassName(String
factoryClassName) {
setProperty(LEDGER_MANAGER_FACTORY_CLASS, factoryClassName);
}
+ /**
+ * Get Ledger Manager Factory Class Name.
+ *
+ * @return ledger manager factory class name.
+ */
+ public String getLedgerManagerFactoryClassName() {
+ return getString(LEDGER_MANAGER_FACTORY_CLASS);
+ }
+
/**
* Set Ledger Manager Factory Class.
*
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerFactory.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerFactory.java
index c6f2817d9..4fa2546be 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerFactory.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerFactory.java
@@ -19,6 +19,7 @@
import java.io.IOException;
import java.util.List;
+import java.util.Objects;
import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.conf.AbstractConfiguration;
import org.apache.bookkeeper.meta.LayoutManager.LedgerLayoutExistsException;
@@ -127,7 +128,10 @@ public static LedgerManagerFactory newLedgerManagerFactory(
try {
factoryClass = conf.getLedgerManagerFactoryClass();
} catch (Exception e) {
- throw new IOException("Failed to get ledger manager factory class
from configuration : ", e);
+ factoryClass = attemptToResolveShadedLedgerManagerFactory(
+ conf,
+ conf.getLedgerManagerFactoryClassName(),
+ e);
}
String ledgerRootPath = conf.getZkLedgersRootPath();
@@ -178,7 +182,9 @@ public static LedgerManagerFactory newLedgerManagerFactory(
}
// handle V2 layout case
- if (factoryClass != null &&
!layout.getManagerFactoryClass().equals(factoryClass.getName())
+ if (factoryClass != null
+ && !isSameLedgerManagerFactory(
+ conf, layout.getManagerFactoryClass(),
factoryClass.getName())
&&
conf.getProperty(AbstractConfiguration.LEDGER_MANAGER_FACTORY_DISABLE_CLASS_CHECK)
== null) {
// Disable should ONLY happen during compatibility testing.
@@ -194,8 +200,10 @@ public static LedgerManagerFactory newLedgerManagerFactory(
}
factoryClass = theCls.asSubclass(LedgerManagerFactory.class);
} catch (ClassNotFoundException cnfe) {
- throw new IOException("Failed to instantiate ledger manager
factory "
- + layout.getManagerFactoryClass());
+ factoryClass = attemptToResolveShadedLedgerManagerFactory(
+ conf,
+ layout.getManagerFactoryClass(),
+ cnfe);
}
}
// instantiate a factory
@@ -203,6 +211,67 @@ public static LedgerManagerFactory newLedgerManagerFactory(
return lmFactory.initialize(conf, layoutManager,
layout.getManagerVersion());
}
+ private static String normalizedLedgerManagerFactoryClassName(String
factoryClass,
+ String
shadedClassPrefix,
+ boolean
isShadedClassAllowed) {
+ if (isShadedClassAllowed) {
+ if (null == factoryClass || null == shadedClassPrefix) {
+ return factoryClass;
+ } else {
+ return factoryClass.replace(shadedClassPrefix, "");
+ }
+ } else {
+ return factoryClass;
+ }
+ }
+
+ private static boolean isSameLedgerManagerFactory(
+ AbstractConfiguration<?> conf, String leftFactoryClass, String
rightFactoryClass) {
+ leftFactoryClass = normalizedLedgerManagerFactoryClassName(
+ leftFactoryClass,
+ conf.getShadedLedgerManagerFactoryClassPrefix(),
+ conf.isShadedLedgerManagerFactoryClassAllowed());
+ rightFactoryClass = normalizedLedgerManagerFactoryClassName(
+ rightFactoryClass,
+ conf.getShadedLedgerManagerFactoryClassPrefix(),
+ conf.isShadedLedgerManagerFactoryClassAllowed());
+ return Objects.equals(leftFactoryClass, rightFactoryClass);
+ }
+
+ private static Class<? extends LedgerManagerFactory>
attemptToResolveShadedLedgerManagerFactory(
+ AbstractConfiguration<?> conf, String lmfClassName, Throwable
cause) throws IOException {
+ if (conf.isShadedLedgerManagerFactoryClassAllowed()) {
+ String shadedPrefix =
conf.getShadedLedgerManagerFactoryClassPrefix();
+ log.warn("Failed to instantiate ledger manager factory {}, trying
its shaded class {}{}",
+ lmfClassName, shadedPrefix, lmfClassName);
+ // try shading class
+ try {
+ return resolveShadedLedgerManagerFactory(lmfClassName,
shadedPrefix);
+ } catch (ClassNotFoundException cnfe) {
+ throw new IOException("Failed to instantiate ledger manager
factory "
+ + lmfClassName + " and its shaded class " + shadedPrefix +
lmfClassName, cnfe);
+ }
+ } else {
+ throw new IOException("Failed to instantiate ledger manager
factory "
+ + lmfClassName, cause);
+ }
+ }
+
+ private static Class<? extends LedgerManagerFactory>
resolveShadedLedgerManagerFactory(String lmfClassName,
+
String shadedClassPrefix)
+ throws ClassNotFoundException, IOException {
+ if (null == lmfClassName) {
+ return null;
+ } else {
+ // this is to address the issue when using the dlog shaded jar
+ Class<?> theCls = Class.forName(shadedClassPrefix + lmfClassName);
+ if (!LedgerManagerFactory.class.isAssignableFrom(theCls)) {
+ throw new IOException("Wrong shaded ledger manager factory : "
+ shadedClassPrefix + lmfClassName);
+ }
+ return theCls.asSubclass(LedgerManagerFactory.class);
+ }
+ }
+
/**
* Creates the new layout and stores in zookeeper and returns the
* LedgerManagerFactory instance.
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerManager.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerManager.java
index 6a02e2c6b..8a0ac6c12 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerManager.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerManager.java
@@ -115,7 +115,7 @@ public void testBadConf() throws Exception {
} catch (Exception e) {
LOG.error("Received exception", e);
assertTrue("Invalid exception",
- e.getMessage().contains("Failed to get ledger manager
factory class from configuration"));
+ e.getMessage().contains("Failed to instantiate ledger
manager factory DoesNotExist"));
}
}
diff --git a/shaded/distributedlog-core-shaded/pom.xml
b/shaded/distributedlog-core-shaded/pom.xml
index ba147ab5c..96d4864ae 100644
--- a/shaded/distributedlog-core-shaded/pom.xml
+++ b/shaded/distributedlog-core-shaded/pom.xml
@@ -188,9 +188,13 @@
<shadedPattern>dlshade.org.rocksdb</shadedPattern>
</relocation>
<!-- bookkeeper -->
+ <relocation>
+ <pattern>com.scurrilous.circe</pattern>
+ <shadedPattern>dlshade.com.scurrilous.circe</shadedPattern>
+ </relocation>
<relocation>
<pattern>org.apache.bookkeeper</pattern>
- <shadedPattern>org.apache.bookkeeper</shadedPattern>
+ <shadedPattern>dlshade.org.apache.bookkeeper</shadedPattern>
</relocation>
<!-- distributedlog -->
<relocation>
diff --git
a/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java
b/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java
index 8bb48cd98..45be333af 100644
---
a/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java
+++
b/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java
@@ -18,13 +18,38 @@
*/
package org.apache.bookkeeper.tests.shaded;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import dlshade.org.apache.bookkeeper.conf.AbstractConfiguration;
+import dlshade.org.apache.bookkeeper.conf.ServerConfiguration;
+import dlshade.org.apache.bookkeeper.meta.AbstractZkLedgerManagerFactory;
+import dlshade.org.apache.bookkeeper.meta.HierarchicalLedgerManagerFactory;
+import dlshade.org.apache.bookkeeper.meta.LayoutManager;
+import dlshade.org.apache.bookkeeper.meta.LedgerLayout;
+import dlshade.org.apache.bookkeeper.meta.LedgerManagerFactory;
+import dlshade.org.apache.bookkeeper.util.ReflectionUtils;
+import java.io.IOException;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
/**
* Test whether the distributedlog-core-shaded jar is generated correctly.
*/
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ AbstractZkLedgerManagerFactory.class, ReflectionUtils.class
})
public class DistributedLogCoreShadedJarTest {
@Test(expected = ClassNotFoundException.class)
@@ -58,21 +83,37 @@ public void testZooKeeperShadedPath() throws Exception {
Class.forName("dlshade.org.apache.zookeeper.ZooKeeper");
}
- @Test
+ @Test(expected = ClassNotFoundException.class)
public void testBookKeeperCommon() throws Exception {
Class.forName("org.apache.bookkeeper.util.OrderedSafeExecutor");
assertTrue(true);
}
@Test
+ public void testBookKeeperCommonShade() throws Exception {
+
Class.forName("dlshade.org.apache.bookkeeper.util.OrderedSafeExecutor");
+ assertTrue(true);
+ }
+
+ @Test(expected = ClassNotFoundException.class)
public void testBookKeeperProto() throws Exception {
Class.forName("org.apache.bookkeeper.proto.BookkeeperProtocol");
- assertTrue(true);
}
@Test
+ public void testBookKeeperProtoShade() throws Exception {
+
Class.forName("dlshade.org.apache.bookkeeper.proto.BookkeeperProtocol");
+ assertTrue(true);
+ }
+
+ @Test(expected = ClassNotFoundException.class)
public void testCirceChecksum() throws Exception {
Class.forName("com.scurrilous.circe.checksum.Crc32cIntChecksum");
+ }
+
+ @Test
+ public void testCirceChecksumShade() throws Exception {
+
Class.forName("dlshade.com.scurrilous.circe.checksum.Crc32cIntChecksum");
assertTrue(true);
}
@@ -93,4 +134,73 @@ public void testDistributedLogCore() throws Exception {
Class.forName("org.apache.distributedlog.api.AsyncLogReader");
assertTrue(true);
}
+
+ @Test
+ public void
testShadeLedgerManagerFactoryWithoutConfiguredLedgerManagerClass() throws
Exception {
+ testShadeLedgerManagerFactoryAllowed(
+ null,
+ true);
+ }
+
+
+ @Test
+ public void
testShadeLedgerManagerFactoryWithConfiguredLedgerManagerClass() throws
Exception {
+ testShadeLedgerManagerFactoryAllowed(
+ "org.apache.bookkeeper.meta.HirerchicalLedgerManagerFactory",
+ true);
+ }
+
+ @Test
+ public void
testShadeLedgerManagerFactoryDisallowedWithoutConfiguredLedgerManagerClass()
throws Exception {
+ testShadeLedgerManagerFactoryAllowed(
+ null,
+ false);
+ }
+
+
+ @Test
+ public void
testShadeLedgerManagerFactoryDisallowedWithConfiguredLedgerManagerClass()
throws Exception {
+ testShadeLedgerManagerFactoryAllowed(
+ "org.apache.bookkeeper.meta.HirerchicalLedgerManagerFactory",
+ false);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void testShadeLedgerManagerFactoryAllowed(String factoryClassName,
+ boolean allowShaded)
throws Exception {
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setAllowShadedLedgerManagerFactoryClass(allowShaded);
+ conf.setLedgerManagerFactoryClassName(factoryClassName);
+
+ LayoutManager manager = mock(LayoutManager.class);
+ LedgerLayout layout = new LedgerLayout(
+ "org.apache.bookkeeper.meta.HierarchicalLedgerManagerFactory",
+ HierarchicalLedgerManagerFactory.CUR_VERSION);
+ when(manager.readLedgerLayout()).thenReturn(layout);
+
+ LedgerManagerFactory factory = mock(LedgerManagerFactory.class);
+ when(factory.initialize(any(AbstractConfiguration.class),
same(manager), anyInt()))
+ .thenReturn(factory);
+ PowerMockito.mockStatic(ReflectionUtils.class);
+ when(ReflectionUtils.newInstance(any(Class.class)))
+ .thenReturn(factory);
+
+ try {
+ LedgerManagerFactory result =
AbstractZkLedgerManagerFactory.newLedgerManagerFactory(
+ conf, manager);
+ if (allowShaded) {
+ assertSame(factory, result);
+ verify(factory, times(1))
+ .initialize(any(AbstractConfiguration.class),
same(manager), anyInt());
+ } else {
+ fail("Should fail to instantiate ledger manager factory if
allowShaded is false");
+ }
+ } catch (IOException ioe) {
+ if (allowShaded) {
+ fail("Should not fail to instantiate ledger manager factory is
allowShaded is true");
+ } else {
+ assertTrue(ioe.getCause() instanceof ClassNotFoundException);
+ }
+ }
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
