INTERNAL: Limit bulk get keys size

Author: cheesecrust

src/main/java/net/spy/memcached/protocol/ascii/AsciiMemcachedNodeImpl.java

@@ -31,6 +31,9 @@
  * Memcached node for the ASCII protocol.
  */
 public final class AsciiMemcachedNodeImpl extends TCPMemcachedNodeImpl {
+
+  private static final int GET_BULK_CHUNK_SIZE = 200;
+
   public AsciiMemcachedNodeImpl(String name,
                                 SocketAddress sa,
                                 int bufSize, BlockingQueue<Operation> rq,
@@ -42,34 +45,39 @@ public AsciiMemcachedNodeImpl(String name,
 
   @Override
   protected void optimize() {
-    // make sure there are at least two get operations in a row before
-    // attempting to optimize them.
     Operation nxtOp = writeQ.peek();
-    if (nxtOp instanceof GetOperation && nxtOp.getAPIType() != APIType.MGET) {
-      optimizedOp = writeQ.remove();
-      nxtOp = writeQ.peek();
-      if (nxtOp instanceof GetOperation && nxtOp.getAPIType() != APIType.MGET) {
-        OptimizedGetImpl og = new OptimizedGetImpl(
-                (GetOperation) optimizedOp);
-        optimizedOp = og;
+    if (!(nxtOp instanceof GetOperation) || nxtOp.getAPIType() == APIType.MGET ||
+            ((GetOperation) nxtOp).getKeys().size() > GET_BULK_CHUNK_SIZE) {
+      return;
+    }
 
-        do {
-          GetOperationImpl o = (GetOperationImpl) writeQ.remove();
-          if (!o.isCancelled()) {
-            og.addOperation(o);
-          }
-          nxtOp = writeQ.peek();
-        } while (nxtOp instanceof GetOperation &&
-                nxtOp.getAPIType() != APIType.MGET);
+    int cnt = ((GetOperation) nxtOp).getKeys().size();
+    optimizedOp = writeQ.remove();
+    nxtOp = writeQ.peek();
+    OptimizedGetImpl og = null;
 
-        // Initialize the new mega get
-        optimizedOp.initialize();
-        assert optimizedOp.getState() == OperationState.WRITE_QUEUED;
-        ProxyCallback pcb = (ProxyCallback) og.getCallback();
-        getLogger().debug("Set up %s with %s keys and %s callbacks",
-                this, pcb.numKeys(), pcb.numCallbacks());
+    while (nxtOp instanceof GetOperation && nxtOp.getAPIType() != APIType.MGET) {
+      if (og == null) {
+        og = (OptimizedGetImpl) optimizedOp;
+        assert og != null;
+      }
+      cnt += ((GetOperation) nxtOp).getKeys().size();
+      if (cnt > GET_BULK_CHUNK_SIZE) {
+        break;
       }
+      GetOperationImpl currentOp = (GetOperationImpl) writeQ.remove();
+      if (!currentOp.isCancelled()) {
+        og.addOperation(currentOp);
+      }
+      nxtOp = writeQ.peek();
+    }
+    // Initialize the new mega get
+    if (og != null) {
+      optimizedOp.initialize();
+      assert optimizedOp.getState() == OperationState.WRITE_QUEUED;
+      ProxyCallback pcb = (ProxyCallback) optimizedOp.getCallback();
+      getLogger().debug("Set up %s with %s keys and %s callbacks",
+              this, pcb.numKeys(), pcb.numCallbacks());
     }
   }
-
 }

src/test/java/net/spy/memcached/OptimizeTest.java

@@ -0,0 +1,57 @@
+package net.spy.memcached;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Future;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class OptimizeTest {
+
+  private ArcusClientPool client;
+  private List<String> keys;
+
+  @BeforeEach
+  void setUp() throws Exception {
+    ConnectionFactoryBuilder builder = new ConnectionFactoryBuilder();
+    builder.setShouldOptimize(true);
+    // Get a connection with the get optimization.
+    client = ArcusClient.createArcusClientPool("127.0.0.1:2181", "test", builder, 1);
+
+    keys = new ArrayList<>(10000);
+    for (int i = 0; i < 100; i++) {
+      keys.add("k" + i);
+      Boolean b = client.set(keys.get(i), 0, "value" + i).get();
+      Assertions.assertEquals(true, b);
+    }
+  }
+
+  @Test
+  void testParallelGet() throws Throwable {
+    List<Future<Object>> results = new ArrayList<>(10000);
+    for (int i = 0; i < 100; i++) {
+      results.add(client.asyncGet(keys.get(i)));
+    }
+
+    for (int i = 0; i < 100; i++) {
+      Object o = results.get(i).get();
+      Assertions.assertEquals("value" + i, o);
+    }
+  }
+
+  @Test
+  void testOptimizedOneOperation() throws Throwable {
+    List<Future<Object>> results = new ArrayList<>(10000);
+    for (int i = 0; i < 2; i++) {
+      results.add(client.asyncGet(keys.get(i)));
+    }
+    client.set(keys.get(0), 0, "value0");
+
+    for (int i = 0; i < 2; i++) {
+      Object o = results.get(i).get();
+      Assertions.assertEquals("value" + i, o);
+    }
+  }
+}