Merge pull request #4012 from vespa-engine/balder/omit-sessionid-from-cache-key

1 - Ignores the session key when creating the cache key.

6 files changed, 73 insertions, 29 deletions

diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java
index eb05bdc672c..1513cf2213c 100644
--- a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java
+++ b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java
@@ -26,8 +26,12 @@ import java.util.List;
  */
 public class QueryPacket extends Packet {
 
-    private Query query;
+    final private Query query;
     private QueryPacketData queryPacketData;
+    int sessionOffset = 0; // Start of sessionKey ignore section for cache key
+    int sessionSize = 0; // Length of sessionKey ignore section for cache key
+    int ignoreableOffset = 0; // Start of (hits/offset/timestamp) ignore section for cache key
+    int ignoreableSize = 0;  // Length of (hits/offset/timestamp) ignore section for cache key
 
     private QueryPacket(Query query) {
         this.query = query;
@@ -73,6 +77,9 @@ public class QueryPacket extends Packet {
         return new byte[0];
     }
 
+    private int getSessionKeySkipLength() {
+        return (sessionSize > 0) ? sessionSize + 4 : 0;
+    }
     /**
      * Returns an opaque cache key for the query represented by this
      * (pre-serialized) packet.
@@ -86,35 +93,41 @@ public class QueryPacket extends Packet {
         // need to fiddle with feature flags to handle a non-existing
         // summary class.
 
-        int skipOffset     = 4;  // offset of offset/hits/timestamp fields
-        int skipLength     = 12; // length of offset/hits/timestamp fields
         byte[] utf8Summary = getSummaryClassAsUtf8();
-        byte[] stripped    = new byte[encodedBody.length - skipLength + utf8Summary.length + 1];
-
-        System.arraycopy(encodedBody, 0, stripped, 0, skipOffset);
-        System.arraycopy(utf8Summary, 0, stripped, skipOffset, utf8Summary.length);
-        stripped[skipOffset + utf8Summary.length] = 0;
-        System.arraycopy(encodedBody, skipOffset + skipLength,
-                         stripped, skipOffset + utf8Summary.length + 1,
-                         encodedBody.length - (skipOffset + skipLength));
+        byte[] stripped    = new byte[encodedBody.length - (ignoreableSize + getSessionKeySkipLength()) + utf8Summary.length + 1];
+
+        System.arraycopy(encodedBody, 0, stripped, 0, ignoreableOffset);
+        stripped[1] = (byte)(stripped[1] & 0x7f);  // Ignor sessionKey feature flag
+        System.arraycopy(utf8Summary, 0, stripped, ignoreableOffset, utf8Summary.length);
+        stripped[ignoreableOffset + utf8Summary.length] = 0;
+
+        // Copy part up to sessionKey
+        System.arraycopy(encodedBody, ignoreableOffset + ignoreableSize,
+                         stripped, ignoreableOffset + utf8Summary.length + 1,
+                         sessionOffset - (ignoreableOffset + ignoreableSize));
+        // Copy part after sessionKey
+        System.arraycopy(encodedBody, sessionOffset + getSessionKeySkipLength(),
+                stripped,  utf8Summary.length + 1 + (sessionOffset -  ignoreableSize),
+                encodedBody.length - (sessionOffset + getSessionKeySkipLength()));
         return stripped;
     }
 
     public void encodeBody(ByteBuffer buffer) {
         queryPacketData = new QueryPacketData();
-        int startOfFieldToSave;
+        final int relativeZero = buffer.position();
 
         boolean sendSessionKey = query.getGroupingSessionCache() || query.getRanking().getQueryCache();
         int featureFlag = getFeatureInt(sendSessionKey);
         buffer.putInt(featureFlag);
 
+        ignoreableOffset = buffer.position() - relativeZero;
         IntegerCompressor.putCompressedPositiveNumber(getOffset(), buffer);
         IntegerCompressor.putCompressedPositiveNumber(getHits(), buffer);
         // store the cutoff time in the tag object, and then do a similar Math.max there
         buffer.putInt(Math.max(50, (int)query.getTimeLeft()));
+        ignoreableSize = buffer.position() - relativeZero - ignoreableOffset;
         buffer.putInt(getFlagInt());
-
-        startOfFieldToSave = buffer.position();
+        int startOfFieldToSave = buffer.position();
         Item.putString(query.getRanking().getProfile(), buffer);
         queryPacketData.setRankProfile(buffer, startOfFieldToSave);
 
@@ -147,8 +160,10 @@ public class QueryPacket extends Packet {
             buffer.put(blob);
         }
 
+        sessionOffset = buffer.position() - relativeZero;
         if (sendSessionKey) {
             Utf8String key = query.getSessionId(true).asUtf8String();
+            sessionSize = key.getByteLength();
             buffer.putInt(key.getByteLength());
             buffer.put(key.getBytes());
         }
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheKey.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheKey.java
index 725ef89eadb..6af0d181695 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheKey.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheKey.java
@@ -14,8 +14,8 @@ import com.yahoo.fs4.QueryPacket;
  * @author  <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
  */
 public class CacheKey {
-    private int hashCode;
-    private byte[] serialized = null;
+    final private int hashCode;
+    final private byte[] serialized;
 
     /**
      * Create a cache key from the query packet.
diff --git a/container-search/src/test/java/com/yahoo/fs4/test/GetDocSumsPacketTestCase.java b/container-search/src/test/java/com/yahoo/fs4/test/GetDocSumsPacketTestCase.java
index 78bf14bae99..b771c64d1a9 100644
--- a/container-search/src/test/java/com/yahoo/fs4/test/GetDocSumsPacketTestCase.java
+++ b/container-search/src/test/java/com/yahoo/fs4/test/GetDocSumsPacketTestCase.java
@@ -6,6 +6,7 @@ import com.yahoo.fs4.GetDocSumsPacket;
 import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
+import com.yahoo.search.query.SessionId;
 import com.yahoo.search.result.Hit;
 import org.junit.Test;
 
@@ -64,11 +65,13 @@ public class GetDocSumsPacketTestCase {
     @Test
     public void requireThatSessionIdIsEncodedAsPropertyWhenUsingSearchSession() throws BufferTooSmallException {
         Result result = new Result(new Query("?query=foo"));
-        result.getQuery().getSessionId(true);  // create session id.
+        SessionId sessionId = result.getQuery().getSessionId(true);  // create session id.
         result.getQuery().getRanking().setQueryCache(true);
         FastHit hit = new FastHit();
         result.hits().add(hit);
-        assertPacket(false, result, new byte[] { 0, 0, 0, -115, 0, 0, 0, -37, 0, 0, 56, 17, 0, 0, 0, 0,
+        ByteBuffer answer = ByteBuffer.allocate(1024);
+        //assertEquals(0, sessionId.asUtf8String().getByteLength());
+        answer.put(new byte[] { 0, 0, 0, (byte)(107+sessionId.asUtf8String().getByteLength()), 0, 0, 0, -37, 0, 0, 56, 17, 0, 0, 0, 0,
                 // query timeout
                 IGNORE, IGNORE, IGNORE, IGNORE,
                 // "default" - rank profile
@@ -78,13 +81,19 @@ public class GetDocSumsPacketTestCase {
                 // 2 property entries
                 0, 0, 0, 2,
                 // rank: sessionId => qrserver.0.XXXXXXXXXXXXX.0
-                0, 0, 0, 4, 'r', 'a', 'n', 'k', 0, 0, 0, 1, 0, 0, 0, 9, 's', 'e', 's', 's', 'i', 'o', 'n', 'I', 'd', 0, 0, 0, 34, 'q', 'r', 's', 'e', 'r', 'v', 'e', 'r', '.',
-                IGNORE, '.', IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, '.', IGNORE, '.','d', 'e', 'f', 'a', 'u', 'l', 't',
+                0, 0, 0, 4, 'r', 'a', 'n', 'k', 0, 0, 0, 1, 0, 0, 0, 9, 's', 'e', 's', 's', 'i', 'o', 'n', 'I', 'd'});
+        answer.putInt(sessionId.asUtf8String().getByteLength());
+        answer.put(sessionId.asUtf8String().getBytes());
+        answer.put(new byte [] {
                 // caches: features => true
-                0, 0, 0, 6, 'c', 'a', 'c', 'h', 'e', 's', 0, 0, 0, 1, 0, 0, 0, 5, 'q', 'u', 'e', 'r', 'y', 0, 0, 0, 4, 't', 'r', 'u', 'e',
+                0, 0, 0, 6, 'c', 'a', 'c', 'h', 'e', 's',
+                0, 0, 0, 1, 0, 0, 0, 5, 'q', 'u', 'e', 'r', 'y', 0, 0, 0, 4, 't', 'r', 'u', 'e',
                 // flags
-                0, 0, 0, 2
-        });
+                0, 0, 0, 2});
+        byte [] expected = new byte [answer.position()];
+        answer.flip();
+        answer.get(expected);
+        assertPacket(false, result, expected);
     }
 
     private static void assertPacket(boolean sendQuery, Hit hit, byte[] expected) throws BufferTooSmallException {
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/CacheKeyTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/CacheKeyTestCase.java
index d87d32fa5cf..e7ab1cc137a 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/CacheKeyTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/CacheKeyTestCase.java
@@ -26,4 +26,16 @@ public class CacheKeyTestCase extends junit.framework.TestCase {
         assertEquals(k1, k2);
         assertEquals(k1.hashCode(), k2.hashCode());
     }
+
+    public void testSessionKeyIgnored() {
+        Query a = new Query("/?query=abcd");
+        QueryPacket ap = QueryPacket.create(a);
+        Query b = new Query("/?query=abcd&ranking.queryCache=true");
+        QueryPacket bp = QueryPacket.create(b);
+        CacheKey ak = new CacheKey(ap);
+        CacheKey bk = new CacheKey(bp);
+        assertEquals(ak, bk);
+        assertEquals(ak.hashCode(), bk.hashCode());
+        assertFalse(ap.getQueryPacketData().equals(bp.getQueryPacketData()));
+    }
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
index 5a5e7586a32..75d965c6438 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
@@ -26,6 +26,7 @@ import com.yahoo.search.grouping.GroupingRequest;
 import com.yahoo.search.grouping.request.AllOperation;
 import com.yahoo.search.grouping.request.EachOperation;
 import com.yahoo.search.grouping.request.GroupingOperation;
+import com.yahoo.search.query.SessionId;
 import com.yahoo.search.rendering.RendererRegistry;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.searchchain.Execution;
@@ -295,8 +296,10 @@ public class FastSearcherTestCase {
         byte[] actual = new byte[buf.remaining()];
         buf.get(actual);
 
+        SessionId sessionId = query.getSessionId(false);
         byte IGNORE = 69;
-        byte[] expected = new byte[] { 0, 0, 0, -77, 0, 0, 0, -37, 0, 0, 48, 17, 0, 0, 0, 0,
+        ByteBuffer answer = ByteBuffer.allocate(1024);
+        answer.put(new byte[] { 0, 0, 0, (byte)(145+sessionId.asUtf8String().getByteLength()), 0, 0, 0, -37, 0, 0, 48, 17, 0, 0, 0, 0,
                 // query timeout
                 IGNORE, IGNORE, IGNORE, IGNORE,
                 // "default" - rank profile
@@ -304,15 +307,20 @@ public class FastSearcherTestCase {
                 // 3 property entries (rank, match, caches)
                 0, 0, 0, 3,
                 // rank: sessionId => qrserver.0.XXXXXXXXXXXXX.0
-                0, 0, 0, 4, 'r', 'a', 'n', 'k', 0, 0, 0, 1, 0, 0, 0, 9, 's', 'e', 's', 's', 'i', 'o', 'n', 'I', 'd', 0, 0, 0, 34, 'q', 'r', 's', 'e', 'r', 'v', 'e', 'r', '.',
-                IGNORE, '.', IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, IGNORE, '.', IGNORE, '.','d', 'e', 'f', 'a', 'u', 'l', 't',
+                0, 0, 0, 4, 'r', 'a', 'n', 'k', 0, 0, 0, 1, 0, 0, 0, 9, 's', 'e', 's', 's', 'i', 'o', 'n', 'I', 'd'});
+        answer.putInt(sessionId.asUtf8String().getBytes().length);
+        answer.put(sessionId.asUtf8String().getBytes());
+        answer.put(new byte [] {
                 // match: documentdb.searchdoctype => test
                 0, 0, 0, 5, 'm', 'a', 't', 'c', 'h', 0, 0, 0, 1, 0, 0, 0, 24, 'd', 'o', 'c', 'u', 'm', 'e', 'n', 't', 'd', 'b', '.', 's', 'e', 'a', 'r', 'c', 'h', 'd', 'o', 'c', 't', 'y', 'p', 'e', 0, 0, 0, 4, 't', 'e', 's', 't',
                 // sessionId => qrserver.0.XXXXXXXXXXXXX.0
                 0, 0, 0, 6, 'c', 'a', 'c', 'h', 'e', 's', 0, 0, 0, 1, 0, 0, 0, 5, 'q', 'u', 'e', 'r', 'y', 0, 0, 0, 4, 't', 'r', 'u', 'e',
                 // flags
-                0, 0, 0, 2
-        };
+                0, 0, 0, 2});
+        byte [] expected = new byte [answer.position()];
+        answer.flip();
+        answer.get(expected);
+
         assertEquals(expected.length, actual.length);
         for (int i = 0; i < expected.length; ++i) {
             if (expected[i] == IGNORE) {
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PacketCacheTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PacketCacheTestCase.java
index 257dfcc456d..b35ef0e09cf 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PacketCacheTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PacketCacheTestCase.java
@@ -50,7 +50,7 @@ public class PacketCacheTestCase extends junit.framework.TestCase {
 
         cache.setMaxCacheItemPercentage(50);
 
-        final int keysz = 30;
+        final int keysz = 36;
 
         // first control assumptions
         assertEquals(keysz, key1.byteSize());