From commits-return-2453-archive-asf-public=cust-asf.ponee.io@bookkeeper.apache.org Wed Jan 24 20:05:11 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id 82692180630 for ; Wed, 24 Jan 2018 20:05:11 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 73211160C3C; Wed, 24 Jan 2018 19:05:11 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id E69C0160C39 for ; Wed, 24 Jan 2018 20:05:09 +0100 (CET) Received: (qmail 36859 invoked by uid 500); 24 Jan 2018 19:05:09 -0000 Mailing-List: contact commits-help@bookkeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: bookkeeper-dev@bookkeeper.apache.org Delivered-To: mailing list commits@bookkeeper.apache.org Received: (qmail 36835 invoked by uid 99); 24 Jan 2018 19:05:08 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 24 Jan 2018 19:05:08 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 33D7281655; Wed, 24 Jan 2018 19:05:08 +0000 (UTC) Date: Wed, 24 Jan 2018 19:05:08 +0000 To: "commits@bookkeeper.apache.org" Subject: [bookkeeper] branch master updated: ISSUE #1007: move checksum to proto MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <151682070809.12944.8674544680605345308@gitbox.apache.org> From: sijie@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: bookkeeper X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: a512b737160e7d52ed46964018c51302b2408967 X-Git-Newrev: a764c7f3336d2acb6b15e1b0197b0dc4cff7e5b1 X-Git-Rev: a764c7f3336d2acb6b15e1b0197b0dc4cff7e5b1 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. sijie 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 a764c7f ISSUE #1007: move checksum to proto a764c7f is described below commit a764c7f3336d2acb6b15e1b0197b0dc4cff7e5b1 Author: Arvin AuthorDate: Wed Jan 24 11:05:00 2018 -0800 ISSUE #1007: move checksum to proto Descriptions of the changes in this PR: Refactor checksum to proto Master Issue: #<1007> Author: Arvin Reviewers: Enrico Olivelli , Sijie Guo , Venkateswararao Jujjuri (JV) This closes #1018 from ArvinDevel/issue1007, closes #1007 --- .../org/apache/bookkeeper/client/BookKeeper.java | 20 +++++++++-- .../org/apache/bookkeeper/client/LedgerHandle.java | 15 ++++---- .../apache/bookkeeper/client/LedgerHandleAdv.java | 3 +- .../apache/bookkeeper/client/LedgerRecoveryOp.java | 6 ++-- .../apache/bookkeeper/client/PendingReadLacOp.java | 7 ++-- .../apache/bookkeeper/client/PendingReadOp.java | 1 + .../client/ReadLastConfirmedAndEntryOp.java | 1 + .../bookkeeper/client/ReadLastConfirmedOp.java | 4 +-- .../bookkeeper/client/TryReadLastConfirmedOp.java | 6 ++-- .../bookkeeper/proto/BookieProtoEncoding.java | 2 +- .../checksum}/CRC32CDigestManager.java | 2 +- .../checksum}/CRC32DigestManager.java | 4 +-- .../{client => proto/checksum}/DigestManager.java | 40 ++++++++++++++-------- .../checksum}/MacDigestManager.java | 2 +- .../bookkeeper/proto/checksum/package-info.java | 23 +++++++++++++ .../org/apache/bookkeeper/client/ClientUtil.java | 11 +++--- .../bookkeeper/client/MockBookKeeperTestCase.java | 24 +++++++++++-- .../client/ParallelLedgerRecoveryTest.java | 3 +- .../main/resources/bookkeeper/findbugsExclude.xml | 3 ++ .../checksum}/DigestTypeBenchmark.java | 24 ++++++++----- 20 files changed, 142 insertions(+), 59 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java index 6822a0f..79dd625 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java @@ -71,6 +71,7 @@ import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.net.DNSToSwitchMapping; import org.apache.bookkeeper.proto.BookieClient; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback; +import org.apache.bookkeeper.proto.DataFormats; import org.apache.bookkeeper.stats.Counter; import org.apache.bookkeeper.stats.NullStatsLogger; import org.apache.bookkeeper.stats.OpStatsLogger; @@ -649,11 +650,14 @@ public class BookKeeper implements org.apache.bookkeeper.client.api.BookKeeper { } /** - * There are 2 digest types that can be used for verification. The CRC32 is + * There are 3 digest types that can be used for verification. The CRC32 is * cheap to compute but does not protect against byzantine bookies (i.e., a * bookie might report fake bytes and a matching CRC32). The MAC code is more * expensive to compute, but is protected by a password, i.e., a bookie can't - * report fake bytes with a mathching MAC unless it knows the password + * report fake bytes with a mathching MAC unless it knows the password. + * The CRC32C, which use SSE processor instruction, has better performance than CRC32. + * Legacy DigestType for backward compatibility. If we want to add new DigestType, + * we should add it in here, client.api.DigestType and DigestType in DataFormats.proto. */ public enum DigestType { MAC, CRC32, CRC32C; @@ -670,6 +674,18 @@ public class BookKeeper implements org.apache.bookkeeper.client.api.BookKeeper { throw new IllegalArgumentException("Unable to convert digest type " + digestType); } } + public static DataFormats.LedgerMetadataFormat.DigestType toProtoDigestType(DigestType digestType) { + switch (digestType) { + case MAC: + return DataFormats.LedgerMetadataFormat.DigestType.HMAC; + case CRC32: + return DataFormats.LedgerMetadataFormat.DigestType.CRC32; + case CRC32C: + return DataFormats.LedgerMetadataFormat.DigestType.CRC32C; + default: + throw new IllegalArgumentException("Unable to convert digest type " + digestType); + } + } } ZooKeeper getZkHandle() { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java index e34ec27..ba6b3fc 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java @@ -56,7 +56,6 @@ import org.apache.bookkeeper.client.AsyncCallback.ReadCallback; import org.apache.bookkeeper.client.AsyncCallback.ReadLastConfirmedCallback; import org.apache.bookkeeper.client.BKException.BKIncorrectParameterException; import org.apache.bookkeeper.client.BKException.BKReadException; -import org.apache.bookkeeper.client.BookKeeper.DigestType; import org.apache.bookkeeper.client.SyncCallbackUtils.FutureReadLastConfirmed; import org.apache.bookkeeper.client.SyncCallbackUtils.FutureReadLastConfirmedAndEntry; import org.apache.bookkeeper.client.SyncCallbackUtils.SyncAddCallback; @@ -78,6 +77,8 @@ import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.TimedGenericCallback; import org.apache.bookkeeper.proto.DataFormats.LedgerMetadataFormat.State; import org.apache.bookkeeper.proto.PerChannelBookieClientPool; +import org.apache.bookkeeper.proto.checksum.DigestManager; +import org.apache.bookkeeper.proto.checksum.MacDigestManager; import org.apache.bookkeeper.stats.Counter; import org.apache.bookkeeper.stats.Gauge; import org.apache.bookkeeper.util.OrderedSafeExecutor.OrderedSafeGenericCallback; @@ -137,7 +138,7 @@ public class LedgerHandle implements WriteHandle { } LedgerHandle(BookKeeper bk, long ledgerId, LedgerMetadata metadata, - DigestType digestType, byte[] password, EnumSet writeFlags) + BookKeeper.DigestType digestType, byte[] password, EnumSet writeFlags) throws GeneralSecurityException, NumberFormatException { this.bk = bk; this.metadata = metadata; @@ -162,7 +163,7 @@ public class LedgerHandle implements WriteHandle { this.throttler = null; } - macManager = DigestManager.instantiate(ledgerId, password, digestType); + macManager = DigestManager.instantiate(ledgerId, password, BookKeeper.DigestType.toProtoDigestType(digestType)); // If the password is empty, pass the same random ledger key which is generated by the hash of the empty // password, so that the bookie can avoid processing the keys for each entry @@ -1079,8 +1080,8 @@ public class LedgerHandle implements WriteHandle { @Override public void readLastConfirmedDataComplete(int rc, DigestManager.RecoveryData data) { if (rc == BKException.Code.OK) { - updateLastConfirmed(data.lastAddConfirmed, data.length); - cb.readLastConfirmedComplete(rc, data.lastAddConfirmed, ctx); + updateLastConfirmed(data.getLastAddConfirmed(), data.getLength()); + cb.readLastConfirmedComplete(rc, data.getLastAddConfirmed(), ctx); } else { cb.readLastConfirmedComplete(rc, INVALID_ENTRY_ID, ctx); } @@ -1120,9 +1121,9 @@ public class LedgerHandle implements WriteHandle { @Override public void readLastConfirmedDataComplete(int rc, DigestManager.RecoveryData data) { if (rc == BKException.Code.OK) { - updateLastConfirmed(data.lastAddConfirmed, data.length); + updateLastConfirmed(data.getLastAddConfirmed(), data.getLength()); if (completed.compareAndSet(false, true)) { - cb.readLastConfirmedComplete(rc, data.lastAddConfirmed, ctx); + cb.readLastConfirmedComplete(rc, data.getLastAddConfirmed(), ctx); } } else { if (completed.compareAndSet(false, true)) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java index 435c453..0866db7 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java @@ -32,7 +32,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.PriorityBlockingQueue; import java.util.concurrent.RejectedExecutionException; import org.apache.bookkeeper.client.AsyncCallback.AddCallback; -import org.apache.bookkeeper.client.BookKeeper.DigestType; import org.apache.bookkeeper.client.SyncCallbackUtils.SyncAddCallback; import org.apache.bookkeeper.client.api.WriteAdvHandle; import org.apache.bookkeeper.client.api.WriteFlag; @@ -56,7 +55,7 @@ public class LedgerHandleAdv extends LedgerHandle implements WriteAdvHandle { } LedgerHandleAdv(BookKeeper bk, long ledgerId, LedgerMetadata metadata, - DigestType digestType, byte[] password, EnumSet writeFlags) + BookKeeper.DigestType digestType, byte[] password, EnumSet writeFlags) throws GeneralSecurityException, NumberFormatException { super(bk, ledgerId, metadata, digestType, password, writeFlags); pendingAddOps = new PriorityBlockingQueue(10, new PendingOpsComparator()); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java index 6ea21b6..3956c68 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java @@ -23,9 +23,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import org.apache.bookkeeper.client.AsyncCallback.AddCallback; import org.apache.bookkeeper.client.AsyncCallback.CloseCallback; -import org.apache.bookkeeper.client.DigestManager.RecoveryData; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryListener; +import org.apache.bookkeeper.proto.checksum.DigestManager.RecoveryData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -110,8 +110,8 @@ class LedgerRecoveryOp implements ReadEntryListener, AddCallback { public void readLastConfirmedDataComplete(int rc, RecoveryData data) { if (rc == BKException.Code.OK) { synchronized (lh) { - lh.lastAddPushed = lh.lastAddConfirmed = data.lastAddConfirmed; - lh.length = data.length; + lh.lastAddPushed = lh.lastAddConfirmed = data.getLastAddConfirmed(); + lh.length = data.getLength(); startEntryToRead = endEntryToRead = lh.lastAddConfirmed; } // keep a copy of ledger metadata before proceeding diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadLacOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadLacOp.java index 07cbb0e..ecdbc5d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadLacOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadLacOp.java @@ -20,8 +20,8 @@ package org.apache.bookkeeper.client; import io.netty.buffer.ByteBuf; import org.apache.bookkeeper.client.BKException.BKDigestMatchException; -import org.apache.bookkeeper.client.DigestManager.RecoveryData; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadLacCallback; +import org.apache.bookkeeper.proto.checksum.DigestManager.RecoveryData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -102,8 +102,9 @@ class PendingReadLacOp implements ReadLacCallback { // Extract lac from last entry on the disk RecoveryData recoveryData = lh.macManager.verifyDigestAndReturnLastConfirmed(lastEntryBuffer); - if (recoveryData.lastAddConfirmed > maxLac) { - maxLac = recoveryData.lastAddConfirmed; + long recoveredLac = recoveryData.getLastAddConfirmed(); + if (recoveredLac > maxLac) { + maxLac = recoveredLac; } heardValidResponse = true; } catch (BKDigestMatchException e) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java index 2e59e35..041ccd1 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java @@ -42,6 +42,7 @@ import org.apache.bookkeeper.common.util.SafeRunnable; import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallbackCtx; +import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.stats.Counter; import org.apache.bookkeeper.stats.OpStatsLogger; import org.apache.bookkeeper.util.MathUtils; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOp.java index 07cb318..aba3a17 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOp.java @@ -33,6 +33,7 @@ import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.proto.BookieProtocol; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks; import org.apache.bookkeeper.proto.ReadLastConfirmedAndEntryContext; +import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.util.MathUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java index 0ae93cc..d0e116e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java @@ -21,9 +21,9 @@ import io.netty.buffer.ByteBuf; import io.netty.util.ReferenceCountUtil; import org.apache.bookkeeper.client.BKException.BKDigestMatchException; -import org.apache.bookkeeper.client.DigestManager.RecoveryData; import org.apache.bookkeeper.proto.BookieProtocol; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback; +import org.apache.bookkeeper.proto.checksum.DigestManager.RecoveryData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -88,7 +88,7 @@ class ReadLastConfirmedOp implements ReadEntryCallback { if (rc == BKException.Code.OK) { try { RecoveryData recoveryData = lh.macManager.verifyDigestAndReturnLastConfirmed(buffer); - if (recoveryData.lastAddConfirmed > maxRecoveredData.lastAddConfirmed) { + if (recoveryData.getLastAddConfirmed() > maxRecoveredData.getLastAddConfirmed()) { maxRecoveredData = recoveryData; } heardValidResponse = true; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TryReadLastConfirmedOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TryReadLastConfirmedOp.java index 39026fa..bcaf231 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TryReadLastConfirmedOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/TryReadLastConfirmedOp.java @@ -19,10 +19,10 @@ package org.apache.bookkeeper.client; import io.netty.buffer.ByteBuf; -import org.apache.bookkeeper.client.DigestManager.RecoveryData; import org.apache.bookkeeper.client.ReadLastConfirmedOp.LastConfirmedDataCallback; import org.apache.bookkeeper.proto.BookieProtocol; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback; +import org.apache.bookkeeper.proto.checksum.DigestManager.RecoveryData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,9 +72,9 @@ class TryReadLastConfirmedOp implements ReadEntryCallback { RecoveryData recoveryData = lh.macManager.verifyDigestAndReturnLastConfirmed(buffer); if (LOG.isTraceEnabled()) { LOG.trace("Received lastAddConfirmed (lac={}, length={}) from bookie({}) for (lid={}).", - recoveryData.lastAddConfirmed, recoveryData.length, bookieIndex, ledgerId); + recoveryData.getLastAddConfirmed(), recoveryData.getLength(), bookieIndex, ledgerId); } - if (recoveryData.lastAddConfirmed > maxRecoveredData.lastAddConfirmed) { + if (recoveryData.getLastAddConfirmed() > maxRecoveredData.getLastAddConfirmed()) { maxRecoveredData = recoveryData; // callback immediately cb.readLastConfirmedDataComplete(BKException.Code.OK, maxRecoveredData); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java index 0423f76..33bb675 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java @@ -39,8 +39,8 @@ import java.io.IOException; import java.security.NoSuchAlgorithmException; import java.util.List; -import org.apache.bookkeeper.client.MacDigestManager; import org.apache.bookkeeper.proto.BookieProtocol.PacketHeader; +import org.apache.bookkeeper.proto.checksum.MacDigestManager; import org.apache.bookkeeper.util.DoubleByteBuf; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java similarity index 98% rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java rename to bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java index 86394cc..ca7fdec 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java @@ -1,4 +1,4 @@ -package org.apache.bookkeeper.client; +package org.apache.bookkeeper.proto.checksum; /* * Licensed to the Apache Software Foundation (ASF) under one diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32DigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32DigestManager.java similarity index 96% rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32DigestManager.java rename to bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32DigestManager.java index 25a0f61..1f06640 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32DigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32DigestManager.java @@ -1,4 +1,4 @@ -package org.apache.bookkeeper.client; +package org.apache.bookkeeper.proto.checksum; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -18,9 +18,7 @@ package org.apache.bookkeeper.client; * limitations under the License. */ - import io.netty.buffer.ByteBuf; - import java.util.zip.CRC32; class CRC32DigestManager extends DigestManager { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java similarity index 87% rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java rename to bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java index 21c9ae7..67386a6 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java @@ -1,4 +1,4 @@ -package org.apache.bookkeeper.client; +package org.apache.bookkeeper.proto.checksum; /** * Licensed to the Apache Software Foundation (ASF) under one @@ -25,7 +25,8 @@ import io.netty.buffer.Unpooled; import java.security.GeneralSecurityException; import org.apache.bookkeeper.client.BKException.BKDigestMatchException; -import org.apache.bookkeeper.client.BookKeeper.DigestType; +import org.apache.bookkeeper.client.LedgerHandle; +import org.apache.bookkeeper.proto.DataFormats.LedgerMetadataFormat.DigestType; import org.apache.bookkeeper.util.DoubleByteBuf; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,14 +35,14 @@ import org.slf4j.LoggerFactory; * This class takes an entry, attaches a digest to it and packages it with relevant * data so that it can be shipped to the bookie. On the return side, it also * gets a packet, checks that the digest matches, and extracts the original entry - * for the packet. Currently 2 types of digests are supported: MAC (based on SHA-1) and CRC32 + * for the packet. Currently 3 types of digests are supported: MAC (based on SHA-1) and CRC32 and CRC32C. */ -abstract class DigestManager { +public abstract class DigestManager { private static final Logger logger = LoggerFactory.getLogger(DigestManager.class); - static final int METADATA_LENGTH = 32; - static final int LAC_METADATA_LENGTH = 16; + public static final int METADATA_LENGTH = 32; + public static final int LAC_METADATA_LENGTH = 16; long ledgerId; @@ -62,10 +63,10 @@ abstract class DigestManager { macCodeLength = getMacCodeLength(); } - static DigestManager instantiate(long ledgerId, byte[] passwd, DigestType digestType) + public static DigestManager instantiate(long ledgerId, byte[] passwd, DigestType digestType) throws GeneralSecurityException { switch(digestType) { - case MAC: + case HMAC: return new MacDigestManager(ledgerId, passwd); case CRC32: return new CRC32DigestManager(ledgerId); @@ -169,7 +170,7 @@ abstract class DigestManager { } - long verifyDigestAndReturnLac(ByteBuf dataReceived) throws BKDigestMatchException{ + public long verifyDigestAndReturnLac(ByteBuf dataReceived) throws BKDigestMatchException{ if ((LAC_METADATA_LENGTH + macCodeLength) > dataReceived.readableBytes()) { logger.error("Data received is smaller than the minimum for this digest type." + " Either the packet it corrupt, or the wrong digest is configured. " @@ -210,25 +211,36 @@ abstract class DigestManager { * @return * @throws BKDigestMatchException */ - ByteBuf verifyDigestAndReturnData(long entryId, ByteBuf dataReceived) + public ByteBuf verifyDigestAndReturnData(long entryId, ByteBuf dataReceived) throws BKDigestMatchException { verifyDigest(entryId, dataReceived); dataReceived.readerIndex(METADATA_LENGTH + macCodeLength); return dataReceived; } - static class RecoveryData { - long lastAddConfirmed; - long length; + /** + * A representation of RecoveryData. + */ + public static final class RecoveryData { + final long lastAddConfirmed; + final long length; public RecoveryData(long lastAddConfirmed, long length) { this.lastAddConfirmed = lastAddConfirmed; this.length = length; } + public long getLastAddConfirmed() { + return lastAddConfirmed; + } + + public long getLength() { + return length; + } + } - RecoveryData verifyDigestAndReturnLastConfirmed(ByteBuf dataReceived) throws BKDigestMatchException { + public RecoveryData verifyDigestAndReturnLastConfirmed(ByteBuf dataReceived) throws BKDigestMatchException { verifyDigest(dataReceived); dataReceived.readerIndex(8); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/MacDigestManager.java similarity index 98% rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java rename to bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/MacDigestManager.java index fe67700..d897a9e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/MacDigestManager.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.bookkeeper.client; +package org.apache.bookkeeper.proto.checksum; import static com.google.common.base.Charsets.UTF_8; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/package-info.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/package-info.java new file mode 100644 index 0000000..7c5ba38 --- /dev/null +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/package-info.java @@ -0,0 +1,23 @@ +/* + * 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. + */ + +/** + * Classes related to the Bookkeeper checksum. + */ +package org.apache.bookkeeper.proto.checksum; diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ClientUtil.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ClientUtil.java index c0400ee..25842f1 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ClientUtil.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ClientUtil.java @@ -17,21 +17,24 @@ */ package org.apache.bookkeeper.client; +import static org.apache.bookkeeper.proto.DataFormats.LedgerMetadataFormat.DigestType.CRC32; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import java.security.GeneralSecurityException; +import org.apache.bookkeeper.proto.checksum.DigestManager; /** * Client utilities. */ public class ClientUtil { public static ByteBuf generatePacket(long ledgerId, long entryId, long lastAddConfirmed, - long length, byte[] data) { + long length, byte[] data) throws GeneralSecurityException { return generatePacket(ledgerId, entryId, lastAddConfirmed, length, data, 0, data.length); } - public static ByteBuf generatePacket(long ledgerId, long entryId, long lastAddConfirmed, - long length, byte[] data, int offset, int len) { - CRC32DigestManager dm = new CRC32DigestManager(ledgerId); + public static ByteBuf generatePacket(long ledgerId, long entryId, long lastAddConfirmed, long length, + byte[] data, int offset, int len) throws GeneralSecurityException { + DigestManager dm = DigestManager.instantiate(ledgerId, new byte[2], CRC32); return dm.computeDigestAndPackageForSending(entryId, lastAddConfirmed, length, Unpooled.wrappedBuffer(data, offset, len)); } 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 6fc92b7..1ebfb8f 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 @@ -27,6 +27,7 @@ import static org.mockito.Mockito.when; import com.google.common.base.Optional; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -49,6 +50,8 @@ import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.proto.BookieClient; import org.apache.bookkeeper.proto.BookieProtocol; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks; +import org.apache.bookkeeper.proto.DataFormats.LedgerMetadataFormat.DigestType; +import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.stats.NullStatsLogger; import org.apache.bookkeeper.util.OrderedSafeExecutor; import org.junit.After; @@ -333,7 +336,12 @@ public abstract class MockBookKeeperTestCase { long entryId = (Long) args[3]; executor.submitOrdered(ledgerId, () -> { - DigestManager macManager = new CRC32DigestManager(ledgerId); + DigestManager macManager = null; + try { + macManager = DigestManager.instantiate(ledgerId, new byte[2], DigestType.CRC32); + } catch (GeneralSecurityException gse){ + LOG.error("Initialize macManager fail", gse); + } fencedLedgers.add(ledgerId); MockEntry mockEntry = getMockLedgerEntry(ledgerId, bookieSocketAddress, entryId); if (mockEntry != null) { @@ -363,7 +371,12 @@ public abstract class MockBookKeeperTestCase { (BookkeeperInternalCallbacks.ReadEntryCallback) args[3]; executor.submitOrdered(ledgerId, () -> { - DigestManager macManager = new CRC32DigestManager(ledgerId); + DigestManager macManager = null; + try { + macManager = DigestManager.instantiate(ledgerId, new byte[2], DigestType.CRC32); + } catch (GeneralSecurityException gse){ + LOG.error("Initialize macManager fail", gse); + } MockEntry mockEntry = getMockLedgerEntry(ledgerId, bookieSocketAddress, entryId); if (mockEntry != null) { LOG.info("readEntry - found mock entry {}@{} at {}", entryId, ledgerId, bookieSocketAddress); @@ -387,7 +400,12 @@ public abstract class MockBookKeeperTestCase { throws BKException.BKDigestMatchException { ByteBuf toSendCopy = Unpooled.copiedBuffer(toSend); toSendCopy.resetReaderIndex(); - DigestManager macManager = new CRC32DigestManager(ledgerId); + DigestManager macManager = null; + try { + macManager = DigestManager.instantiate(ledgerId, new byte[2], DigestType.CRC32); + } catch (GeneralSecurityException gse){ + LOG.error("Initialize macManager fail", gse); + } ByteBuf content = macManager.verifyDigestAndReturnData(entryId, toSendCopy); byte[] entry = new byte[content.readableBytes()]; content.readBytes(entry); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java index 48a1828..085a248 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ParallelLedgerRecoveryTest.java @@ -52,6 +52,7 @@ import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.LedgerMetadataListener; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.Processor; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.WriteCallback; +import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.test.BookKeeperClusterTestCase; import org.apache.bookkeeper.versioning.Version; import org.apache.zookeeper.AsyncCallback.VoidCallback; @@ -605,7 +606,7 @@ public class ParallelLedgerRecoveryTest extends BookKeeperClusterTestCase { @Override public void readLastConfirmedDataComplete(int rc, DigestManager.RecoveryData data) { rcHolder.set(rc); - lacHolder.set(data.lastAddConfirmed); + lacHolder.set(data.getLastAddConfirmed()); doneLatch.countDown(); } }).initiate(); diff --git a/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml b/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml index 0328f7d..aede153 100644 --- a/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml +++ b/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml @@ -27,6 +27,9 @@ + + + diff --git a/microbenchmarks/src/main/java/org/apache/bookkeeper/client/DigestTypeBenchmark.java b/microbenchmarks/src/main/java/org/apache/bookkeeper/proto/checksum/DigestTypeBenchmark.java similarity index 95% rename from microbenchmarks/src/main/java/org/apache/bookkeeper/client/DigestTypeBenchmark.java rename to microbenchmarks/src/main/java/org/apache/bookkeeper/proto/checksum/DigestTypeBenchmark.java index bda4da5..339f5d2 100644 --- a/microbenchmarks/src/main/java/org/apache/bookkeeper/client/DigestTypeBenchmark.java +++ b/microbenchmarks/src/main/java/org/apache/bookkeeper/proto/checksum/DigestTypeBenchmark.java @@ -19,13 +19,14 @@ * */ -package org.apache.bookkeeper.client; +package org.apache.bookkeeper.proto.checksum; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufAllocator; +import io.netty.buffer.Unpooled; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; - -import io.netty.buffer.ByteBufAllocator; -import org.apache.bookkeeper.client.BookKeeper.DigestType; +import org.apache.bookkeeper.proto.DataFormats.LedgerMetadataFormat.DigestType; import org.apache.bookkeeper.util.DoubleByteBuf; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; @@ -42,28 +43,30 @@ import org.openjdk.jmh.annotations.TearDown; import org.openjdk.jmh.annotations.Threads; import org.openjdk.jmh.annotations.Warmup; -import io.netty.buffer.ByteBuf; -import io.netty.buffer.Unpooled; - /** * Microbenchmarks for different digest type * getting started: * 1. http://tutorials.jenkov.com/java-performance/jmh.html * 2. http://hg.openjdk.java.net/code-tools/jmh/file/tip/jmh-samples/src/main/java/org/openjdk/jmh/samples/ * 3. google - * * To run: * build project from command line. * execute ./run.sh */ public class DigestTypeBenchmark { + /** + * BufferType. + */ public enum BufferType { ARRAY_BACKED, NOT_ARRAY_BACKED, BYTE_BUF_DEFAULT_ALLOC } + /** + * Digest. + */ public enum Digest { MAC, CRC32, @@ -76,6 +79,9 @@ public class DigestTypeBenchmark { return b; } + /** + * MyState. + */ @State(Scope.Thread) public static class MyState { @@ -106,7 +112,7 @@ public class DigestTypeBenchmark { password, DigestType.CRC32C); mac = DigestManager.instantiate(ThreadLocalRandom.current().nextLong(0, Long.MAX_VALUE), - password, DigestType.MAC); + password, DigestType.HMAC); digestBuf = Unpooled.buffer(getDigestManager(digest).getMacCodeLength()); -- To stop receiving notification emails like this one, please contact sijie@apache.org.