You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/03/19 02:23:13 UTC
[GitHub] jiazhai closed pull request #1252: Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
jiazhai closed pull request #1252: Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
URL: https://github.com/apache/bookkeeper/pull/1252
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/client/LedgerOpenOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
index 9a68b0a22..16b577b65 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java
@@ -130,7 +130,11 @@ public void operationComplete(int rc, LedgerMetadata metadata) {
}
final byte[] passwd;
- DigestType digestType = enableDigestAutodetection
+
+ // we should use digest type from metadata *ONLY* when:
+ // 1) digest type is stored in metadata
+ // 2) `autodetection` is enabled
+ DigestType digestType = enableDigestAutodetection && metadata.hasPassword()
? fromApiDigestType(metadata.getDigestType())
: suggestedDigestType;
@@ -149,7 +153,9 @@ public void operationComplete(int rc, LedgerMetadata metadata) {
openComplete(BKException.Code.UnauthorizedAccessException, null);
return;
}
- if (digestType != fromApiDigestType(metadata.getDigestType())) {
+ // if `digest auto detection` is enabled, ignore the suggested digest type, this allows digest type
+ // changes. e.g. moving from `crc32` to `crc32c`.
+ if (suggestedDigestType != fromApiDigestType(metadata.getDigestType()) && !enableDigestAutodetection) {
LOG.error("Provided digest does not match that in metadata");
openComplete(BKException.Code.DigestMatchException, null);
return;
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
index 6511f929f..64fbbe6fa 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
@@ -239,10 +239,12 @@ public ClientConfiguration setThrottleValue(int throttle) {
* <p>Ignores provided digestType, if enabled and uses one from ledger metadata instead.
* Incompatible with ledger created by bookie versions < 4.2
*
+ * <p>It is turned on by default since 4.7.
+ *
* @return flag to enable/disable autodetection of digest type.
*/
public boolean getEnableDigestTypeAutodetection() {
- return getBoolean(ENABLE_DIGEST_TYPE_AUTODETECTION, false);
+ return getBoolean(ENABLE_DIGEST_TYPE_AUTODETECTION, true);
}
/**
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
index 344f5ec82..2ada48ba7 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
@@ -105,9 +105,19 @@ public void testConstructionNotConnectedExplicitZk() throws Exception {
* it provides the wrong password or wrong digest.
*/
@Test
- public void testBookkeeperPassword() throws Exception {
+ public void testBookkeeperDigestPasswordWithAutoDetection() throws Exception {
+ testBookkeeperDigestPassword(true);
+ }
+
+ @Test
+ public void testBookkeeperDigestPasswordWithoutAutoDetection() throws Exception {
+ testBookkeeperDigestPassword(false);
+ }
+
+ void testBookkeeperDigestPassword(boolean autodetection) throws Exception {
ClientConfiguration conf = new ClientConfiguration();
conf.setZkServers(zkUtil.getZooKeeperConnectString());
+ conf.setEnableDigestTypeAutodetection(autodetection);
BookKeeper bkc = new BookKeeper(conf);
DigestType digestCorrect = digestType;
@@ -136,9 +146,14 @@ public void testBookkeeperPassword() throws Exception {
// try open with bad digest
try {
bkc.openLedger(id, digestBad, passwdCorrect);
- fail("Shouldn't be able to open with bad digest");
+ if (!autodetection) {
+ fail("Shouldn't be able to open with bad digest");
+ }
} catch (BKException.BKDigestMatchException bke) {
// correct behaviour
+ if (autodetection) {
+ fail("Should not throw digest match exception if `autodetection` is enabled");
+ }
}
// try open with both bad
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
index b7cc373d9..e686742c5 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
@@ -152,7 +152,7 @@ public void setup() throws Exception {
when(bk.getBookieClient()).thenReturn(bookieClient);
when(bk.getScheduler()).thenReturn(scheduler);
when(bk.getReadSpeculativeRequestPolicy()).thenReturn(Optional.absent());
- when(bk.getConf()).thenReturn(new ClientConfiguration());
+ mockBookKeeperGetConf(new ClientConfiguration());
when(bk.getStatsLogger()).thenReturn(nullStatsLogger);
when(bk.getLedgerManager()).thenReturn(ledgerManager);
when(bk.getLedgerIdGenerator()).thenReturn(ledgerIdGenerator);
@@ -170,6 +170,10 @@ public void setup() throws Exception {
setupBookieClientAddEntry();
}
+ protected void mockBookKeeperGetConf(ClientConfiguration conf) {
+ when(bk.getConf()).thenReturn(conf);
+ }
+
protected NullStatsLogger setupLoggers() {
NullStatsLogger nullStatsLogger = NullStatsLogger.INSTANCE;
when(bk.getOpenOpLogger()).thenReturn(nullStatsLogger.getOpStatsLogger("mock"));
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperApiTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperApiTest.java
index fd30f820a..eadabfb9a 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperApiTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperApiTest.java
@@ -30,6 +30,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import io.netty.buffer.Unpooled;
import java.nio.ByteBuffer;
@@ -43,6 +44,7 @@
import org.apache.bookkeeper.client.BKException.BKNoSuchLedgerExistsException;
import org.apache.bookkeeper.client.BKException.BKUnauthorizedAccessException;
import org.apache.bookkeeper.client.MockBookKeeperTestCase;
+import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.util.LoggerOutput;
import org.junit.Rule;
import org.junit.Test;
@@ -197,8 +199,21 @@ public void testLedgerDigests() throws Exception {
}
- @Test(expected = BKDigestMatchException.class)
- public void testOpenLedgerDigestUnmatched() throws Exception {
+ @Test
+ public void testOpenLedgerDigestUnmatchedWhenAutoDetectionEnabled() throws Exception {
+ testOpenLedgerDigestUnmatched(true);
+ }
+
+ @Test
+ public void testOpenLedgerDigestUnmatchedWhenAutoDetectionDisabled() throws Exception {
+ testOpenLedgerDigestUnmatched(false);
+ }
+
+ private void testOpenLedgerDigestUnmatched(boolean autodetection) throws Exception {
+ ClientConfiguration conf = new ClientConfiguration();
+ conf.setEnableDigestTypeAutodetection(autodetection);
+ mockBookKeeperGetConf(conf);
+
long lId;
try (WriteHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
@@ -215,6 +230,13 @@ public void testOpenLedgerDigestUnmatched() throws Exception {
.withPassword(password)
.withLedgerId(lId)
.execute())) {
+ if (!autodetection) {
+ fail("Should fail to open read handle if digest type auto detection is disabled.");
+ }
+ } catch (BKDigestMatchException bme) {
+ if (autodetection) {
+ fail("Should not fail to open read handle if digest type auto detection is enabled.");
+ }
}
}
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services