zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From eolive...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3560: Add response cache to serve get children (2) requests.
Date Mon, 18 Nov 2019 08:05:44 GMT
This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 945167c  ZOOKEEPER-3560: Add response cache to serve get children (2) requests.
945167c is described below

commit 945167c338d4a26c18bdaf49fb9000186a5f7025
Author: Michael Han <lhan@twitter.com>
AuthorDate: Mon Nov 18 09:05:38 2019 +0100

    ZOOKEEPER-3560: Add response cache to serve get children (2) requests.
    
    ZOOKEEPER-3180 introduces response cache but it only covers getData requests. This commit
is to extend the response cache based on the infrastructure set up by ZOOKEEPER-3180 to so
the response of get children requests can also be served out of cache. Some design decisions:
    
    * Only OpCode.getChildren2 is supported, as OpCode.getChildren does not have associated
stats and current cache infra relies on stats to invalidate cache.
    
    * The children list is stored in a separate response cache object so it does not pollute
the existing data cache that's serving getData requests, and this separation also allows potential
separate tuning of each cache based on workload characteristics.
    
    * As a result of cache object separation, new server metrics is added to measure cache
hit / miss for get children requests, that's separated from get data requests.
    
    Similar as ZOOKEEPER-3180, the get children response cache is enabled by default, with
a default cache size of 400, and can be disabled (together with get data response cache.).
    
    Author: Michael Han <lhan@twitter.com>
    Author: Michael Han <hanm@apache.org>
    
    Reviewers: eolivelli@apache.org, andor@apache.org, enixon@apache.org
    
    Closes #1098 from hanm/cache
---
 .../src/main/resources/markdown/zookeeperAdmin.md  |  10 +++
 .../org/apache/zookeeper/server/DumbWatcher.java   |   3 +-
 .../zookeeper/server/FinalRequestProcessor.java    |  33 ++++---
 .../org/apache/zookeeper/server/NIOServerCnxn.java |  31 ++-----
 .../apache/zookeeper/server/NettyServerCnxn.java   |   5 +-
 .../org/apache/zookeeper/server/ResponseCache.java |  28 +++---
 .../org/apache/zookeeper/server/ServerCnxn.java    |  57 ++++++++++--
 .../org/apache/zookeeper/server/ServerMetrics.java |   9 +-
 .../apache/zookeeper/server/ZooKeeperServer.java   |  16 +++-
 .../apache/zookeeper/server/MockServerCnxn.java    |   3 +-
 .../apache/zookeeper/test/ResponseCacheTest.java   | 100 +++++++++++++++++++--
 11 files changed, 230 insertions(+), 65 deletions(-)

diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 662355d..272731c 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -728,6 +728,16 @@ property, when available, is noted below.
     by default with a value of 400, set to 0 or a negative
     integer to turn the feature off.
 
+* *maxGetChildrenResponseCacheSize* :
+    (Java system property: **zookeeper.maxGetChildrenResponseCacheSize**)
+    **New in 3.6.0:**
+    Similar to **maxResponseCacheSize**, but applies to get children
+    requests. The metrics **response_packet_get_children_cache_hits**
+    and **response_packet_get_children_cache_misses** can be used to tune
+    this value to a given workload. The feature is turned on
+    by default with a value of 400, set to 0 or a negative
+    integer to turn the feature off.
+
 * *autopurge.snapRetainCount* :
     (No Java system property)
     **New in 3.4.0:**
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
index 63da494..9b9672a 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
@@ -61,7 +61,8 @@ public class DumbWatcher extends ServerCnxn {
     }
 
     @Override
-    public void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey, Stat stat)
throws IOException {
+    public void sendResponse(ReplyHeader h, Record r, String tag,
+                             String cacheKey, Stat stat, int opCode) throws IOException {
     }
 
     @Override
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
index 0ffa5af..ee758b2 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
@@ -585,19 +585,32 @@ public class FinalRequestProcessor implements RequestProcessor {
         updateStats(request, lastOp, lastZxid);
 
         try {
-            if (request.type == OpCode.getData && path != null && rsp !=
null) {
-                // Serialized read responses could be cached by the connection object.
-                // Cache entries are identified by their path and last modified zxid,
-                // so these values are passed along with the response.
-                GetDataResponse getDataResponse = (GetDataResponse) rsp;
+            if (path == null || rsp == null) {
+                cnxn.sendResponse(hdr, rsp, "response");
+            } else {
+                int opCode = request.type;
                 Stat stat = null;
-                if (getDataResponse.getStat() != null) {
-                    stat = getDataResponse.getStat();
+                // Serialized read and get children responses could be cached by the connection
+                // object. Cache entries are identified by their path and last modified zxid,
+                // so these values are passed along with the response.
+                switch (opCode) {
+                    case OpCode.getData : {
+                        GetDataResponse getDataResponse = (GetDataResponse) rsp;
+                        stat = getDataResponse.getStat();
+                        cnxn.sendResponse(hdr, rsp, "response", path, stat, opCode);
+                        break;
+                    }
+                    case OpCode.getChildren2 : {
+                        GetChildren2Response getChildren2Response = (GetChildren2Response)
rsp;
+                        stat = getChildren2Response.getStat();
+                        cnxn.sendResponse(hdr, rsp, "response", path, stat, opCode);
+                        break;
+                    }
+                    default:
+                        cnxn.sendResponse(hdr, rsp, "response");
                 }
-                cnxn.sendResponse(hdr, rsp, "response", path, stat);
-            } else {
-                cnxn.sendResponse(hdr, rsp, "response");
             }
+
             if (request.type == OpCode.closeSession) {
                 cnxn.sendCloseSession();
             }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java
index 9157174..d0591e5 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxn.java
@@ -35,6 +35,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.jute.BinaryInputArchive;
 import org.apache.jute.Record;
 import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.proto.ReplyHeader;
@@ -663,31 +664,10 @@ public class NIOServerCnxn extends ServerCnxn {
 
     private static final ByteBuffer packetSentinel = ByteBuffer.allocate(0);
 
-    /**
-     * Serializes a ZooKeeper response and enqueues it for sending.
-     *
-     * Serializes client response parts and enqueues them into outgoing queue.
-     *
-     * If both cache key and last modified zxid are provided, the serialized
-     * response is caсhed under the provided key, the last modified zxid is
-     * stored along with the value. A cache entry is invalidated if the
-     * provided last modified zxid is more recent than the stored one.
-     *
-     * Attention: this function is not thread safe, due to caching not being
-     * thread safe.
-     *
-     * @param h reply header
-     * @param r reply payload, can be null
-     * @param tag Jute serialization tag, can be null
-     * @param cacheKey key for caching the serialized payload. a null value
-     *     prvents caching
-     * @param stat stat information for the the reply payload, used
-     *     for cache invalidation. a value of 0 prevents caching.
-     */
     @Override
-    public void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey, Stat stat)
{
+    public void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey, Stat stat,
int opCode) {
         try {
-            sendBuffer(serialize(h, r, tag, cacheKey, stat));
+            sendBuffer(serialize(h, r, tag, cacheKey, stat, opCode));
             decrOutstandingAndCheckThrottle(h);
         } catch (Exception e) {
             LOG.warn("Unexpected exception. Destruction averted.", e);
@@ -712,7 +692,10 @@ public class NIOServerCnxn extends ServerCnxn {
         // Convert WatchedEvent to a type that can be sent over the wire
         WatcherEvent e = event.getWrapper();
 
-        sendResponse(h, e, "notification", null, null);
+        // The last parameter OpCode here is used to select the response cache.
+        // Passing OpCode.error (with a value of -1) means we don't care, as we don't need
+        // response cache on delivering watcher events.
+        sendResponse(h, e, "notification", null, null, ZooDefs.OpCode.error);
     }
 
     /*
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java
index ca89672..670ad83 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java
@@ -167,13 +167,14 @@ public class NettyServerCnxn extends ServerCnxn {
     }
 
     @Override
-    public void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey, Stat stat)
throws IOException {
+    public void sendResponse(ReplyHeader h, Record r, String tag,
+                             String cacheKey, Stat stat, int opCode) throws IOException {
         // cacheKey and stat are used in caching, which is not
         // implemented here. Implementation example can be found in NIOServerCnxn.
         if (closingChannel || !channel.isOpen()) {
             return;
         }
-        sendBuffer(serialize(h, r, tag, cacheKey, stat));
+        sendBuffer(serialize(h, r, tag, cacheKey, stat, opCode));
         decrOutstandingAndCheckThrottle(h);
     }
 
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java
index 7e728b5..4a76a0f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java
@@ -22,23 +22,31 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @SuppressWarnings("serial")
 public class ResponseCache {
+    private static final Logger LOG = LoggerFactory.getLogger(ResponseCache.class);
 
     // Magic number chosen to be "big enough but not too big"
-    private static final int DEFAULT_RESPONSE_CACHE_SIZE = 400;
-
+    public static final int DEFAULT_RESPONSE_CACHE_SIZE = 400;
+    private final int cacheSize;
     private static class Entry {
-
         public Stat stat;
         public byte[] data;
-
     }
 
-    private Map<String, Entry> cache = Collections.synchronizedMap(new LRUCache<String,
Entry>(getResponseCacheSize()));
+    private final Map<String, Entry> cache;
 
-    public ResponseCache() {
+    public ResponseCache(int cacheSize) {
+        this.cacheSize = cacheSize;
+        cache = Collections.synchronizedMap(new LRUCache<>(cacheSize));
+        LOG.info("Response cache size is initialized with value {}.", cacheSize);
+    }
+
+    public int getCacheSize() {
+        return cacheSize;
     }
 
     public void put(String path, byte[] data, Stat stat) {
@@ -62,12 +70,8 @@ public class ResponseCache {
         }
     }
 
-    private static int getResponseCacheSize() {
-        return Integer.getInteger("zookeeper.maxResponseCacheSize", DEFAULT_RESPONSE_CACHE_SIZE);
-    }
-
-    public static boolean isEnabled() {
-        return getResponseCacheSize() > 0;
+    public boolean isEnabled() {
+        return cacheSize > 0;
     }
 
     private static class LRUCache<K, V> extends LinkedHashMap<K, V> {
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
index bf5a6c7..95f0534 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
@@ -40,8 +40,10 @@ import org.apache.jute.Record;
 import org.apache.zookeeper.Quotas;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZooDefs.OpCode;
 import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.metrics.Counter;
 import org.apache.zookeeper.proto.ReplyHeader;
 import org.apache.zookeeper.proto.RequestHeader;
 import org.slf4j.Logger;
@@ -161,10 +163,34 @@ public abstract class ServerCnxn implements Stats, Watcher {
 
     public abstract void close(DisconnectReason reason);
 
-    public abstract void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey,
Stat stat) throws IOException;
+    /**
+     * Serializes a ZooKeeper response and enqueues it for sending.
+     *
+     * Serializes client response parts and enqueues them into outgoing queue.
+     *
+     * If both cache key and last modified zxid are provided, the serialized
+     * response is caсhed under the provided key, the last modified zxid is
+     * stored along with the value. A cache entry is invalidated if the
+     * provided last modified zxid is more recent than the stored one.
+     *
+     * Attention: this function is not thread safe, due to caching not being
+     * thread safe.
+     *
+     * @param h reply header
+     * @param r reply payload, can be null
+     * @param tag Jute serialization tag, can be null
+     * @param cacheKey Key for caching the serialized payload. A null value prevents caching.
+     * @param stat Stat information for the the reply payload, used for cache invalidation.
+     *             A value of 0 prevents caching.
+     * @param opCode The op code appertains to the corresponding request of the response,
+     *               used to decide which cache (e.g. read response cache,
+     *               list of children response cache, ...) object to look up to when applicable.
+     */
+    public abstract void sendResponse(ReplyHeader h, Record r, String tag,
+                                      String cacheKey, Stat stat, int opCode) throws IOException;
 
     public void sendResponse(ReplyHeader h, Record r, String tag) throws IOException {
-        sendResponse(h, r, tag, null, null);
+        sendResponse(h, r, tag, null, null, -1);
     }
 
     protected byte[] serializeRecord(Record record) throws IOException {
@@ -174,11 +200,30 @@ public abstract class ServerCnxn implements Stats, Watcher {
         return baos.toByteArray();
     }
 
-    protected ByteBuffer[] serialize(ReplyHeader h, Record r, String tag, String cacheKey,
Stat stat) throws IOException {
+    protected ByteBuffer[] serialize(ReplyHeader h, Record r, String tag,
+                                     String cacheKey, Stat stat, int opCode) throws IOException
{
         byte[] header = serializeRecord(h);
         byte[] data = null;
         if (r != null) {
-            ResponseCache cache = zkServer.getReadResponseCache();
+            ResponseCache cache = null;
+            Counter cacheHit = null, cacheMiss = null;
+            switch (opCode) {
+                case OpCode.getData : {
+                    cache = zkServer.getReadResponseCache();
+                    cacheHit = ServerMetrics.getMetrics().RESPONSE_PACKET_CACHE_HITS;
+                    cacheMiss = ServerMetrics.getMetrics().RESPONSE_PACKET_CACHE_MISSING;
+                    break;
+                }
+                case OpCode.getChildren2 : {
+                    cache = zkServer.getGetChildrenResponseCache();
+                    cacheHit = ServerMetrics.getMetrics().RESPONSE_PACKET_GET_CHILDREN_CACHE_HITS;
+                    cacheMiss = ServerMetrics.getMetrics().RESPONSE_PACKET_GET_CHILDREN_CACHE_MISSING;
+                    break;
+                }
+                default:
+                    // op codes where response cache is not supported.
+            }
+
             if (cache != null && stat != null && cacheKey != null &&
!cacheKey.endsWith(Quotas.statNode)) {
                 // Use cache to get serialized data.
                 //
@@ -189,9 +234,9 @@ public abstract class ServerCnxn implements Stats, Watcher {
                     // Cache miss, serialize the response and put it in cache.
                     data = serializeRecord(r);
                     cache.put(cacheKey, data, stat);
-                    ServerMetrics.getMetrics().RESPONSE_PACKET_CACHE_MISSING.add(1);
+                    cacheMiss.add(1);
                 } else {
-                    ServerMetrics.getMetrics().RESPONSE_PACKET_CACHE_HITS.add(1);
+                    cacheHit.add(1);
                 }
             } else {
                 data = serializeRecord(r);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
index 1f9855c..10bf444 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
@@ -95,7 +95,6 @@ public final class ServerMetrics {
         NODE_CHANGED_WATCHER = metricsContext.getSummary("node_changed_watch_count", DetailLevel.BASIC);
         NODE_CHILDREN_WATCHER = metricsContext.getSummary("node_children_watch_count", DetailLevel.BASIC);
 
-
         /*
          * Number of dead watchers in DeadWatcherListener
          */
@@ -106,6 +105,8 @@ public final class ServerMetrics {
 
         RESPONSE_PACKET_CACHE_HITS = metricsContext.getCounter("response_packet_cache_hits");
         RESPONSE_PACKET_CACHE_MISSING = metricsContext.getCounter("response_packet_cache_misses");
+        RESPONSE_PACKET_GET_CHILDREN_CACHE_HITS = metricsContext.getCounter("response_packet_get_children_cache_hits");
+        RESPONSE_PACKET_GET_CHILDREN_CACHE_MISSING = metricsContext.getCounter("response_packet_get_children_cache_misses");
 
         ENSEMBLE_AUTH_SUCCESS = metricsContext.getCounter("ensemble_auth_success");
 
@@ -338,8 +339,14 @@ public final class ServerMetrics {
     public final Counter DEAD_WATCHERS_QUEUED;
     public final Counter DEAD_WATCHERS_CLEARED;
     public final Summary DEAD_WATCHERS_CLEANER_LATENCY;
+
+    /*
+     * Response cache hit and miss metrics.
+     */
     public final Counter RESPONSE_PACKET_CACHE_HITS;
     public final Counter RESPONSE_PACKET_CACHE_MISSING;
+    public final Counter RESPONSE_PACKET_GET_CHILDREN_CACHE_HITS;
+    public final Counter RESPONSE_PACKET_GET_CHILDREN_CACHE_MISSING;
 
     /**
      * Learner handler quorum packet metrics.
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
index 05bf82e..90ad6af 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -163,6 +163,7 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
     private FileTxnSnapLog txnLogFactory = null;
     private ZKDatabase zkDb;
     private ResponseCache readResponseCache;
+    private ResponseCache getChildrenResponseCache;
     private final AtomicLong hzxid = new AtomicLong(0);
     public static final Exception ok = new Exception("No prob");
     protected RequestProcessor firstProcessor;
@@ -217,6 +218,9 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
     public static final int DEFAULT_STARTING_BUFFER_SIZE = 1024;
     public static final int intBufferStartingSizeBytes;
 
+    public static final String GET_DATA_RESPONSE_CACHE_SIZE = "zookeeper.maxResponseCacheSize";
+    public static final String GET_CHILDREN_RESPONSE_CACHE_SIZE = "zookeeper.maxGetChildrenResponseCacheSize";
+
     static {
         long configuredFlushDelay = Long.getLong(FLUSH_DELAY, 0);
         setFlushDelay(configuredFlushDelay);
@@ -306,7 +310,13 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
 
         listener = new ZooKeeperServerListenerImpl(this);
 
-        readResponseCache = new ResponseCache();
+        readResponseCache = new ResponseCache(Integer.getInteger(
+            GET_DATA_RESPONSE_CACHE_SIZE,
+            ResponseCache.DEFAULT_RESPONSE_CACHE_SIZE));
+
+        getChildrenResponseCache = new ResponseCache(Integer.getInteger(
+            GET_CHILDREN_RESPONSE_CACHE_SIZE,
+            ResponseCache.DEFAULT_RESPONSE_CACHE_SIZE));
 
         this.initialConfig = initialConfig;
 
@@ -1764,6 +1774,10 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         return isResponseCachingEnabled ? readResponseCache : null;
     }
 
+    public ResponseCache getGetChildrenResponseCache() {
+        return isResponseCachingEnabled ? getChildrenResponseCache : null;
+    }
+
     protected void registerMetrics() {
         MetricsContext rootContext = ServerMetrics.getMetrics().getMetricsProvider().getRootContext();
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
index 8314545..a82eab3 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
@@ -46,7 +46,8 @@ public class MockServerCnxn extends ServerCnxn {
     }
 
     @Override
-    public void sendResponse(ReplyHeader h, Record r, String tag, String cacheKey, Stat stat)
throws IOException {
+    public void sendResponse(ReplyHeader h, Record r, String tag,
+                             String cacheKey, Stat stat, int opCode) throws IOException {
     }
 
     @Override
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java
index 9831e87..0b27ff5 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java
@@ -21,6 +21,8 @@ package org.apache.zookeeper.test;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.fail;
+import java.util.List;
 import java.util.Map;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.ZooDefs;
@@ -28,6 +30,9 @@ import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.metrics.MetricsUtils;
 import org.apache.zookeeper.server.ServerMetrics;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,6 +41,19 @@ public class ResponseCacheTest extends ClientBase {
 
     protected static final Logger LOG = LoggerFactory.getLogger(ResponseCacheTest.class);
 
+    @Before
+    public void setup() throws Exception {
+        System.setProperty(ZooKeeperServer.GET_DATA_RESPONSE_CACHE_SIZE, "32");
+        System.setProperty(ZooKeeperServer.GET_CHILDREN_RESPONSE_CACHE_SIZE, "64");
+        super.setUp();
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        System.clearProperty(ZooKeeperServer.GET_DATA_RESPONSE_CACHE_SIZE);
+        System.clearProperty(ZooKeeperServer.GET_CHILDREN_RESPONSE_CACHE_SIZE);
+    }
+
     @Test
     public void testResponseCache() throws Exception {
         ZooKeeper zk = createClient();
@@ -48,11 +66,12 @@ public class ResponseCacheTest extends ClientBase {
         }
     }
 
-    private void checkCacheStatus(long expectedHits, long expectedMisses) {
+    private void checkCacheStatus(long expectedHits, long expectedMisses,
+                                  String cacheHitMetricsName, String cacheMissMetricsName)
{
 
         Map<String, Object> metrics = MetricsUtils.currentServerMetrics();
-        assertEquals(expectedHits, metrics.get("response_packet_cache_hits"));
-        assertEquals(expectedMisses, metrics.get("response_packet_cache_misses"));
+        assertEquals(expectedHits, metrics.get(cacheHitMetricsName));
+        assertEquals(expectedMisses, metrics.get(cacheMissMetricsName));
     }
 
     public void performCacheTest(ZooKeeper zk, String path, boolean useCache) throws Exception
{
@@ -64,9 +83,15 @@ public class ResponseCacheTest extends ClientBase {
         long expectedHits = 0;
         long expectedMisses = 0;
 
-        serverFactory.getZooKeeperServer().setResponseCachingEnabled(useCache);
+        ZooKeeperServer zks = serverFactory.getZooKeeperServer();
+        zks.setResponseCachingEnabled(useCache);
         LOG.info("caching: {}", useCache);
 
+        if (useCache) {
+            assertEquals(zks.getReadResponseCache().getCacheSize(), 32);
+            assertEquals(zks.getGetChildrenResponseCache().getCacheSize(), 64);
+        }
+
         byte[] writeData = "test1".getBytes();
         zk.create(path, writeData, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT, writeStat);
         for (int i = 0; i < reads; ++i) {
@@ -78,7 +103,8 @@ public class ResponseCacheTest extends ClientBase {
             expectedMisses += 1;
             expectedHits += reads - 1;
         }
-        checkCacheStatus(expectedHits, expectedMisses);
+        checkCacheStatus(expectedHits, expectedMisses, "response_packet_cache_hits",
+                "response_packet_cache_misses");
 
         writeData = "test2".getBytes();
         writeStat = zk.setData(path, writeData, -1);
@@ -91,7 +117,8 @@ public class ResponseCacheTest extends ClientBase {
             expectedMisses += 1;
             expectedHits += reads - 1;
         }
-        checkCacheStatus(expectedHits, expectedMisses);
+        checkCacheStatus(expectedHits, expectedMisses, "response_packet_cache_hits",
+                "response_packet_cache_misses");
 
         // Create a child beneath the tested node. This won't change the data of
         // the tested node, but will change it's pzxid. The next read of the tested
@@ -104,7 +131,66 @@ public class ResponseCacheTest extends ClientBase {
         }
         assertArrayEquals(writeData, readData);
         assertNotSame(writeStat, readStat);
-        checkCacheStatus(expectedHits, expectedMisses);
+        checkCacheStatus(expectedHits, expectedMisses, "response_packet_cache_hits",
+                "response_packet_cache_misses");
+
+        ServerMetrics.getMetrics().resetAll();
+        expectedHits = 0;
+        expectedMisses = 0;
+        createPath(path + "/a", zk);
+        createPath(path + "/a/b", zk);
+        createPath(path + "/a/c", zk);
+        createPath(path + "/a/b/d", zk);
+        createPath(path + "/a/b/e", zk);
+        createPath(path + "/a/b/e/f", zk);
+        createPath(path + "/a/b/e/g", zk);
+        createPath(path + "/a/b/e/h", zk);
+
+        checkPath(path + "/a", zk, 2);
+        checkPath(path + "/a/b", zk, 2);
+        checkPath(path + "/a/c", zk, 0);
+        checkPath(path + "/a/b/d", zk, 0);
+        checkPath(path + "/a/b/e", zk, 3);
+        checkPath(path + "/a/b/e/h", zk, 0);
+
+        if (useCache) {
+            expectedMisses += 6;
+        }
+
+        checkCacheStatus(expectedHits, expectedMisses, "response_packet_get_children_cache_hits",
+                "response_packet_get_children_cache_misses");
+
+        checkPath(path + "/a", zk, 2);
+        checkPath(path + "/a/b", zk, 2);
+        checkPath(path + "/a/c", zk, 0);
+
+        if (useCache) {
+            expectedHits += 3;
+        }
+
+        checkCacheStatus(expectedHits, expectedMisses, "response_packet_get_children_cache_hits",
+                "response_packet_get_children_cache_misses");
+    }
+
+    private void createPath(String path, ZooKeeper zk) throws Exception {
+        zk.create(path, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT,
null);
+    }
+
+    private void checkPath(String path, ZooKeeper zk, int expectedNumberOfChildren) throws
Exception {
+        Stat stat = zk.exists(path, false);
+
+        List<String> c1 = zk.getChildren(path, false);
+        List<String> c2 = zk.getChildren(path, false, stat);
+
+        if (!c1.equals(c2)) {
+            fail("children lists from getChildren()/getChildren2() do not match");
+        }
+
+        assertEquals(c1.size(), expectedNumberOfChildren);
+
+        if (!stat.equals(stat)) {
+            fail("stats from exists()/getChildren2() do not match");
+        }
     }
 
 }


Mime
View raw message