You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by zh...@apache.org on 2018/03/19 02:23:10 UTC
[bookkeeper] branch master updated: Set
`ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
This is an automated email from the ASF dual-hosted git repository.
zhaijia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new c4c8c03 Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
c4c8c03 is described below
commit c4c8c035df04d702d92b4e57b3fe1e5c1b554afc
Author: Sijie Guo <si...@apache.org>
AuthorDate: Mon Mar 19 10:23:00 2018 +0800
Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
Descriptions of the changes in this PR:
*Motivation*
We have recorded digest type in the ledger metadata since BK 4.5. it would be great to let client automatically detect the digest type
on opening ledger. So turn on the autodetection feature by default.
*Result*
digest type autodetection is on by default.
Author: Sijie Guo <si...@apache.org>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>, Matteo Merli <mm...@apache.org>
This closes #1252 from sijie/flip_default_values
---
.../org/apache/bookkeeper/client/LedgerOpenOp.java | 10 +++++++--
.../bookkeeper/conf/ClientConfiguration.java | 4 +++-
.../apache/bookkeeper/client/BookKeeperTest.java | 19 ++++++++++++++--
.../bookkeeper/client/MockBookKeeperTestCase.java | 6 ++++-
.../bookkeeper/client/api/BookKeeperApiTest.java | 26 ++++++++++++++++++++--
5 files changed, 57 insertions(+), 8 deletions(-)
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 9a68b0a..16b577b 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 @@ class LedgerOpenOp implements GenericCallback<LedgerMetadata> {
}
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 @@ class LedgerOpenOp implements GenericCallback<LedgerMetadata> {
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 6511f92..64fbbe6 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 class ClientConfiguration extends AbstractConfiguration<ClientConfigurati
* <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 344f5ec..2ada48b 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 class BookKeeperTest extends BookKeeperClusterTestCase {
* 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 class BookKeeperTest extends BookKeeperClusterTestCase {
// 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 b7cc373..e686742 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 abstract class MockBookKeeperTestCase {
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 abstract class MockBookKeeperTestCase {
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 fd30f82..eadabfb 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.assertEquals;
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.BKLedgerFencedException;
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 class BookKeeperApiTest extends MockBookKeeperTestCase {
}
- @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 class BookKeeperApiTest extends MockBookKeeperTestCase {
.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.");
+ }
}
}
--
To stop receiving notification emails like this one, please contact
zhaijia@apache.org.