Changed type for tookTime from long to Long, to address Sagar's comment

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
opensearch-project · Oct 26, 2023 · 4e57f4c · 4e57f4c
1 parent a9ab327
commit 4e57f4c
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 22 deletions.
diff --git a/server/src/main/java/org/opensearch/search/query/QuerySearchResult.java b/server/src/main/java/org/opensearch/search/query/QuerySearchResult.java
@@ -88,7 +88,7 @@ public final class QuerySearchResult extends SearchPhaseResult {
     private int nodeQueueSize = -1;
 
     private final boolean isNull;
-    private long tookTimeNanos;
+    private Long tookTimeNanos = null;
 
     public QuerySearchResult() {
         this(false);
@@ -367,9 +367,9 @@ public void readFromWithId(ShardSearchContextId id, StreamInput in) throws IOExc
         setShardSearchRequest(in.readOptionalWriteable(ShardSearchRequest::new));
         setRescoreDocIds(new RescoreDocIds(in));
         if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
-            tookTimeNanos = in.readVLong();
+            tookTimeNanos = in.readOptionalLong();
         } else {
-            tookTimeNanos = -1L;
+            tookTimeNanos = null;
         }
     }
 
@@ -414,7 +414,7 @@ public void writeToNoId(StreamOutput out) throws IOException {
         out.writeOptionalWriteable(getShardSearchRequest());
         getRescoreDocIds().writeTo(out);
         if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
-            out.writeVLong(tookTimeNanos); // VLong as took time should always be positive
+            out.writeOptionalLong(tookTimeNanos);
         }
     }
 
@@ -426,7 +426,7 @@ public float getMaxScore() {
         return maxScore;
     }
 
-    public long getTookTimeNanos() {
+    public Long getTookTimeNanos() {
         return tookTimeNanos;
     }
 

diff --git a/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java b/server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java
@@ -1204,7 +1204,7 @@ public void testQuerySearchResultTookTime() throws IOException {
 
         QueryPhase queryPhase = new QueryPhase(delayedQueryPhaseSearcher);
         queryPhase.execute(searchContext);
-        long tookTime = searchContext.queryResult().getTookTimeNanos();
+        Long tookTime = searchContext.queryResult().getTookTimeNanos();
         assertTrue(tookTime >= (long) sleepMillis * 1000000);
         reader.close();
         dir.close();

diff --git a/server/src/test/java/org/opensearch/search/query/QuerySearchResultTests.java b/server/src/test/java/org/opensearch/search/query/QuerySearchResultTests.java
@@ -99,27 +99,35 @@ private static QuerySearchResult createTestInstance() throws Exception {
         if (randomBoolean()) {
             result.aggregations(InternalAggregationsTests.createTestInstance());
         }
-        assertEquals(0, result.getTookTimeNanos());
+        assertNull(result.getTookTimeNanos());
         return result;
     }
 
     public void testSerialization() throws Exception {
-        QuerySearchResult querySearchResult = createTestInstance();
-        QuerySearchResult deserialized = copyWriteable(querySearchResult, namedWriteableRegistry, QuerySearchResult::new);
-        assertEquals(querySearchResult.getContextId().getId(), deserialized.getContextId().getId());
-        assertNull(deserialized.getSearchShardTarget());
-        assertEquals(querySearchResult.topDocs().maxScore, deserialized.topDocs().maxScore, 0f);
-        assertEquals(querySearchResult.topDocs().topDocs.totalHits, deserialized.topDocs().topDocs.totalHits);
-        assertEquals(querySearchResult.from(), deserialized.from());
-        assertEquals(querySearchResult.size(), deserialized.size());
-        assertEquals(querySearchResult.hasAggs(), deserialized.hasAggs());
-        if (deserialized.hasAggs()) {
-            Aggregations aggs = querySearchResult.consumeAggs().expand();
-            Aggregations deserializedAggs = deserialized.consumeAggs().expand();
-            assertEquals(aggs.asList(), deserializedAggs.asList());
+        for (int i = 0; i < 2; i++) {
+            QuerySearchResult querySearchResult = createTestInstance();
+            if (i == 0) {
+                querySearchResult.setTookTimeNanos(1000L); // test both an initialized and an uninitialized null value
+            }
+            QuerySearchResult deserialized = copyWriteable(querySearchResult, namedWriteableRegistry, QuerySearchResult::new);
+            assertEquals(querySearchResult.getContextId().getId(), deserialized.getContextId().getId());
+            assertNull(deserialized.getSearchShardTarget());
+            assertEquals(querySearchResult.topDocs().maxScore, deserialized.topDocs().maxScore, 0f);
+            assertEquals(querySearchResult.topDocs().topDocs.totalHits, deserialized.topDocs().topDocs.totalHits);
+            assertEquals(querySearchResult.from(), deserialized.from());
+            assertEquals(querySearchResult.size(), deserialized.size());
+            assertEquals(querySearchResult.hasAggs(), deserialized.hasAggs());
+            if (deserialized.hasAggs()) {
+                Aggregations aggs = querySearchResult.consumeAggs().expand();
+                Aggregations deserializedAggs = deserialized.consumeAggs().expand();
+                assertEquals(aggs.asList(), deserializedAggs.asList());
+            }
+            assertEquals(querySearchResult.terminatedEarly(), deserialized.terminatedEarly());
+            assertEquals(querySearchResult.getTookTimeNanos(), deserialized.getTookTimeNanos());
+            if (i == 1) {
+                assertNull(deserialized.getTookTimeNanos());
+            }
         }
-        assertEquals(querySearchResult.terminatedEarly(), deserialized.terminatedEarly());
-        assertEquals(querySearchResult.getTookTimeNanos(), deserialized.getTookTimeNanos());
     }
 
     public void testNullResponse() throws Exception {