You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by eo...@apache.org on 2017/05/17 06:57:59 UTC
bookkeeper git commit: BOOKKEEPER-1055: Optimize handling of
masterKey in case it is empty
Repository: bookkeeper
Updated Branches:
refs/heads/master e44c73883 -> 0f16da896
BOOKKEEPER-1055: Optimize handling of masterKey in case it is empty
On each request client and bookies are exchanging the ledger masterKey, which is a 20 bytes MAC digest of the ledger password.
For each request there is a considerable overhead in allocating byte arrays when parsing the add/read requests.
If the client is a passing an empty password, we should optimize the data path to skip all allocations (related to the masterKey) and instead rely on a static byte array.
Author: Matteo Merli <mm...@apache.org>
Author: Matteo Merli <mm...@yahoo-inc.com>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Venkateswararao Jujjuri (JV) <None>
Closes #156 from merlimat/empty-password
Project: http://git-wip-us.apache.org/repos/asf/bookkeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/bookkeeper/commit/0f16da89
Tree: http://git-wip-us.apache.org/repos/asf/bookkeeper/tree/0f16da89
Diff: http://git-wip-us.apache.org/repos/asf/bookkeeper/diff/0f16da89
Branch: refs/heads/master
Commit: 0f16da8961521372b9bec953ab62c8913bb8dc31
Parents: e44c738
Author: Matteo Merli <mm...@apache.org>
Authored: Wed May 17 08:57:37 2017 +0200
Committer: eolivelli <eo...@apache.org>
Committed: Wed May 17 08:57:37 2017 +0200
----------------------------------------------------------------------
.../bookkeeper/bookie/LedgerDescriptorImpl.java | 3 +-
.../apache/bookkeeper/client/LedgerHandle.java | 16 ++++++-
.../bookkeeper/client/MacDigestManager.java | 23 +++++----
.../bookkeeper/proto/BookieProtoEncoding.java | 50 ++++++++++++++++----
4 files changed, 72 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/0f16da89/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
----------------------------------------------------------------------
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
index 6602392..f80bbc8 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
@@ -22,7 +22,6 @@
package org.apache.bookkeeper.bookie;
import io.netty.buffer.ByteBuf;
-import io.netty.buffer.ByteBufUtil;
import java.io.IOException;
import java.util.Arrays;
@@ -51,6 +50,8 @@ public class LedgerDescriptorImpl extends LedgerDescriptor {
@Override
void checkAccess(byte masterKey[]) throws BookieException, IOException {
if (!Arrays.equals(this.masterKey, masterKey)) {
+ LOG.error("[{}] Requested master key {} does not match the cached master key {}", new Object[] {
+ this.ledgerId, Arrays.toString(masterKey), Arrays.toString(this.masterKey) });
throw BookieException.create(BookieException.Code.UnauthorizedAccessException);
}
}
http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/0f16da89/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
----------------------------------------------------------------------
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 4ae4f48..06f1d8c 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
@@ -24,6 +24,7 @@ import static com.google.common.base.Charsets.UTF_8;
import io.netty.buffer.ByteBuf;
import java.security.GeneralSecurityException;
+import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Enumeration;
@@ -92,6 +93,16 @@ public class LedgerHandle implements AutoCloseable {
final Counter lacUpdateHitsCounter;
final Counter lacUpdateMissesCounter;
+ // This empty master key is used when an empty password is provided which is the hash of an empty string
+ private final static byte[] emptyLedgerKey;
+ static {
+ try {
+ emptyLedgerKey = MacDigestManager.genDigest("ledger", new byte[0]);
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
LedgerHandle(BookKeeper bk, long ledgerId, LedgerMetadata metadata,
DigestType digestType, byte[] password)
throws GeneralSecurityException, NumberFormatException {
@@ -117,7 +128,10 @@ public class LedgerHandle implements AutoCloseable {
}
macManager = DigestManager.instantiate(ledgerId, password, digestType);
- this.ledgerKey = MacDigestManager.genDigest("ledger", password);
+
+ // 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
+ this.ledgerKey = password.length > 0 ? MacDigestManager.genDigest("ledger", password) : emptyLedgerKey;
distributionSchedule = new RoundRobinDistributionSchedule(
metadata.getWriteQuorumSize(), metadata.getAckQuorumSize(), metadata.getEnsembleSize());
http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/0f16da89/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
----------------------------------------------------------------------
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
index 920143f..fc526aa 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
@@ -1,5 +1,3 @@
-package org.apache.bookkeeper.client;
-
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -18,11 +16,16 @@ package org.apache.bookkeeper.client;
* limitations under the License.
*/
+package org.apache.bookkeeper.client;
+
+import static com.google.common.base.Charsets.UTF_8;
+
import io.netty.buffer.ByteBuf;
import java.security.GeneralSecurityException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
@@ -30,13 +33,13 @@ import javax.crypto.spec.SecretKeySpec;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static com.google.common.base.Charsets.UTF_8;
-
-class MacDigestManager extends DigestManager {
+public class MacDigestManager extends DigestManager {
private final static Logger LOG = LoggerFactory.getLogger(MacDigestManager.class);
- public static String DIGEST_ALGORITHM = "SHA-1";
- public static String KEY_ALGORITHM = "HmacSHA1";
+ public static final String DIGEST_ALGORITHM = "SHA-1";
+ public static final String KEY_ALGORITHM = "HmacSHA1";
+
+ public static final int MAC_CODE_LENGTH = 20;
final byte[] passwd;
@@ -58,10 +61,10 @@ class MacDigestManager extends DigestManager {
public MacDigestManager(long ledgerId, byte[] passwd) throws GeneralSecurityException {
super(ledgerId);
- this.passwd = passwd;
+ this.passwd = Arrays.copyOf(passwd, passwd.length);
}
- static byte[] genDigest(String pad, byte[] passwd) throws NoSuchAlgorithmException {
+ public static byte[] genDigest(String pad, byte[] passwd) throws NoSuchAlgorithmException {
MessageDigest digest = MessageDigest.getInstance(DIGEST_ALGORITHM);
digest.update(pad.getBytes(UTF_8));
digest.update(passwd);
@@ -70,7 +73,7 @@ class MacDigestManager extends DigestManager {
@Override
int getMacCodeLength() {
- return 20;
+ return MAC_CODE_LENGTH;
}
http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/0f16da89/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
----------------------------------------------------------------------
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 0fed3e5..cd71b26 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
@@ -21,8 +21,10 @@
package org.apache.bookkeeper.proto;
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.util.DoubleByteBuf;
import org.slf4j.Logger;
@@ -37,7 +39,6 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
import io.netty.buffer.ByteBufInputStream;
import io.netty.buffer.ByteBufOutputStream;
-import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandler.Sharable;
import io.netty.channel.ChannelHandlerContext;
@@ -74,6 +75,16 @@ public class BookieProtoEncoding {
static class RequestEnDeCoderPreV3 implements EnDecoder {
final ExtensionRegistry extensionRegistry;
+ //This empty master key is used when an empty password is provided which is the hash of an empty string
+ private final static byte[] emptyPasswordMasterKey;
+ static {
+ try {
+ emptyPasswordMasterKey = MacDigestManager.genDigest("ledger", new byte[0]);
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
RequestEnDeCoderPreV3(ExtensionRegistry extensionRegistry) {
this.extensionRegistry = extensionRegistry;
}
@@ -134,29 +145,27 @@ public class BookieProtoEncoding {
// packet format is different between ADDENTRY and READENTRY
long ledgerId = -1;
long entryId = BookieProtocol.INVALID_ENTRY_ID;
- byte[] masterKey = null;
+
short flags = h.getFlags();
ServerStats.getInstance().incrementPacketsReceived();
switch (h.getOpCode()) {
- case BookieProtocol.ADDENTRY:
- // first read master key
- masterKey = new byte[BookieProtocol.MASTER_KEY_LENGTH];
- packet.readBytes(masterKey, 0, BookieProtocol.MASTER_KEY_LENGTH);
+ case BookieProtocol.ADDENTRY: {
+ byte[] masterKey = readMasterKey(packet);
// Read ledger and entry id without advancing the reader index
ledgerId = packet.getLong(packet.readerIndex());
entryId = packet.getLong(packet.readerIndex() + 8);
return new BookieProtocol.AddRequest(h.getVersion(), ledgerId, entryId, flags, masterKey, packet.retain());
+ }
case BookieProtocol.READENTRY:
ledgerId = packet.readLong();
entryId = packet.readLong();
if ((flags & BookieProtocol.FLAG_DO_FENCING) == BookieProtocol.FLAG_DO_FENCING
&& h.getVersion() >= 2) {
- masterKey = new byte[BookieProtocol.MASTER_KEY_LENGTH];
- packet.readBytes(masterKey, 0, BookieProtocol.MASTER_KEY_LENGTH);
+ byte[] masterKey = readMasterKey(packet);
return new BookieProtocol.ReadRequest(h.getVersion(), ledgerId, entryId, flags, masterKey);
} else {
return new BookieProtocol.ReadRequest(h.getVersion(), ledgerId, entryId, flags);
@@ -170,6 +179,31 @@ public class BookieProtoEncoding {
return packet;
}
+
+ private static byte[] readMasterKey(ByteBuf packet) {
+ byte[] masterKey = null;
+
+ // check if the master key is an empty master key
+ boolean isEmptyKey = true;
+ for (int i = 0; i < BookieProtocol.MASTER_KEY_LENGTH; i++) {
+ if (packet.getByte(packet.readerIndex() + i) != emptyPasswordMasterKey[i]) {
+ isEmptyKey = false;
+ break;
+ }
+ }
+
+ if (isEmptyKey) {
+ // avoid new allocations if incoming master key is empty and use the static master key
+ masterKey = emptyPasswordMasterKey;
+ packet.readerIndex(packet.readerIndex() + BookieProtocol.MASTER_KEY_LENGTH);
+ } else {
+ // Master key is set, we need to copy and check it
+ masterKey = new byte[BookieProtocol.MASTER_KEY_LENGTH];
+ packet.readBytes(masterKey, 0, BookieProtocol.MASTER_KEY_LENGTH);
+ }
+
+ return masterKey;
+ }
}
static class ResponseEnDeCoderPreV3 implements EnDecoder {