Skip to content

Commit db8fe9c

Browse files
authored
ZOOKEEPER-4919: Make ResponseCache a LRU as it supposed to be
Reviewers: tisonkun, kezhuw Author: mikkosc Closes #2243 from mikkosc/fix-lru-cache
1 parent 97a29de commit db8fe9c

File tree

3 files changed

+32
-10
lines changed

3 files changed

+32
-10
lines changed

zookeeper-server/src/main/java/org/apache/zookeeper/server/ResponseCache.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,13 @@ public class ResponseCache {
3333
public static final int DEFAULT_RESPONSE_CACHE_SIZE = 400;
3434
private final int cacheSize;
3535
private static class Entry {
36-
public Stat stat;
37-
public byte[] data;
36+
public final Stat stat;
37+
public final byte[] data;
38+
39+
Entry(Stat stat, byte[] data) {
40+
this.stat = stat;
41+
this.data = data;
42+
}
3843
}
3944

4045
private final Map<String, Entry> cache;
@@ -50,10 +55,7 @@ public int getCacheSize() {
5055
}
5156

5257
public void put(String path, byte[] data, Stat stat) {
53-
Entry entry = new Entry();
54-
entry.data = data;
55-
entry.stat = stat;
56-
cache.put(path, entry);
58+
cache.put(path, new Entry(stat, data));
5759
}
5860

5961
public byte[] get(String key, Stat stat) {
@@ -76,10 +78,10 @@ public boolean isEnabled() {
7678

7779
private static class LRUCache<K, V> extends LinkedHashMap<K, V> {
7880

79-
private int cacheSize;
81+
private final int cacheSize;
8082

8183
LRUCache(int cacheSize) {
82-
super(cacheSize / 4);
84+
super(cacheSize / 4, 0.75f, true);
8385
this.cacheSize = cacheSize;
8486
}
8587

zookeeper-server/src/test/java/org/apache/zookeeper/test/ResponseCacheTest.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ public void performCacheTest(ZooKeeper zk, String path, boolean useCache) throws
7979
Stat writeStat = new Stat();
8080
Stat readStat = new Stat();
8181
byte[] readData = null;
82+
int cacheSize = Integer.getInteger(ZooKeeperServer.GET_DATA_RESPONSE_CACHE_SIZE);
8283
int reads = 10;
8384
long expectedHits = 0;
8485
long expectedMisses = 0;
@@ -88,7 +89,7 @@ public void performCacheTest(ZooKeeper zk, String path, boolean useCache) throws
8889
LOG.info("caching: {}", useCache);
8990

9091
if (useCache) {
91-
assertEquals(zks.getReadResponseCache().getCacheSize(), 32);
92+
assertEquals(zks.getReadResponseCache().getCacheSize(), cacheSize);
9293
assertEquals(zks.getGetChildrenResponseCache().getCacheSize(), 64);
9394
}
9495

@@ -146,15 +147,21 @@ public void performCacheTest(ZooKeeper zk, String path, boolean useCache) throws
146147
createPath(path + "/a/b/e/g", zk);
147148
createPath(path + "/a/b/e/h", zk);
148149

150+
createPath(path + "/x", zk);
151+
for (int i = 0; i < cacheSize * 2; ++i) {
152+
createPath(path + "/x/y" + i, zk);
153+
}
154+
149155
checkPath(path + "/a", zk, 2);
150156
checkPath(path + "/a/b", zk, 2);
151157
checkPath(path + "/a/c", zk, 0);
152158
checkPath(path + "/a/b/d", zk, 0);
153159
checkPath(path + "/a/b/e", zk, 3);
154160
checkPath(path + "/a/b/e/h", zk, 0);
161+
checkPath(path + "/x", zk, cacheSize * 2);
155162

156163
if (useCache) {
157-
expectedMisses += 6;
164+
expectedMisses += 7;
158165
}
159166

160167
checkCacheStatus(expectedHits, expectedMisses, "response_packet_get_children_cache_hits",
@@ -170,6 +177,19 @@ public void performCacheTest(ZooKeeper zk, String path, boolean useCache) throws
170177

171178
checkCacheStatus(expectedHits, expectedMisses, "response_packet_get_children_cache_hits",
172179
"response_packet_get_children_cache_misses");
180+
181+
for (int i = 0; i < cacheSize * 2; ++i) {
182+
checkPath(path + "/a", zk, 2);
183+
checkPath(path + "/x/y" + i, zk, 0);
184+
185+
if (useCache) {
186+
expectedHits += 1;
187+
expectedMisses += 1;
188+
}
189+
190+
checkCacheStatus(expectedHits, expectedMisses, "response_packet_get_children_cache_hits",
191+
"response_packet_get_children_cache_misses");
192+
}
173193
}
174194

175195
private void createPath(String path, ZooKeeper zk) throws Exception {

0 commit comments

Comments
 (0)