[Backport 2.x] Per request phase latency (#10351) (#10797)

* Per request phase latency (#10351) Signed-off-by: David Zane <[email protected]> (cherry picked from commit daf1350) * Update SearchRequest version check Signed-off-by: David Zane <[email protected]> --------- Signed-off-by: David Z <[email protected]> Signed-off-by: David Zane <[email protected]>
opensearch-project · Oct 23, 2023 · 7e12f36 · 7e12f36
1 parent 9cf9e8a
commit 7e12f36
Show file tree

Hide file tree

Showing 14 changed files with 394 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Remote cluster state] Restore global metadata from remote store when local state is lost after quorum loss ([#10404](https://github.com/opensearch-project/OpenSearch/pull/10404))
 - [Remote cluster state] Make index and global metadata upload timeout dynamic cluster settings ([#10814](https://github.com/opensearch-project/OpenSearch/pull/10814))
 - Add search query categorizor ([#10255](https://github.com/opensearch-project/OpenSearch/pull/10255))
+- Per request phase latency ([#10351](https://github.com/opensearch-project/OpenSearch/issues/10351))
 
 ### Dependencies
 - Bumps jetty version to 9.4.52.v20230823 to fix GMS-2023-1857 ([#9822](https://github.com/opensearch-project/OpenSearch/pull/9822))

diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java
@@ -215,6 +215,7 @@ public final void start() {
                     0,
                     0,
                     buildTookInMillis(),
+                    timeProvider.getPhaseTook(),
                     ShardSearchFailure.EMPTY_ARRAY,
                     clusters,
                     null
@@ -662,6 +663,7 @@ protected final SearchResponse buildSearchResponse(
             successfulOps.get(),
             skippedOps.get(),
             buildTookInMillis(),
+            timeProvider.getPhaseTook(),
             failures,
             clusters,
             searchContextId

diff --git a/server/src/main/java/org/opensearch/action/search/MultiSearchRequest.java b/server/src/main/java/org/opensearch/action/search/MultiSearchRequest.java
@@ -277,6 +277,8 @@ public static void readMultiLineFormat(
                         } else if ("cancel_after_time_interval".equals(entry.getKey())
                             || "cancelAfterTimeInterval".equals(entry.getKey())) {
                                 searchRequest.setCancelAfterTimeInterval(nodeTimeValue(value, null));
+                            } else if ("phase_took".equals(entry.getKey())) {
+                                searchRequest.setPhaseTook(nodeBooleanValue(value));
                             } else {
                                 throw new IllegalArgumentException("key [" + entry.getKey() + "] is not supported in the metadata section");
                             }
@@ -374,6 +376,9 @@ public static void writeSearchRequestParams(SearchRequest request, XContentBuild
         if (request.getCancelAfterTimeInterval() != null) {
             xContentBuilder.field("cancel_after_time_interval", request.getCancelAfterTimeInterval().getStringRep());
         }
+        if (request.isPhaseTook() != null) {
+            xContentBuilder.field("phase_took", request.isPhaseTook());
+        }
         xContentBuilder.endObject();
     }
 

diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequest.java b/server/src/main/java/org/opensearch/action/search/SearchRequest.java
@@ -118,6 +118,8 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla
 
     private String pipeline;
 
+    private Boolean phaseTook = null;
+
     public SearchRequest() {
         this.localClusterAlias = null;
         this.absoluteStartMillis = DEFAULT_ABSOLUTE_START_MILLIS;
@@ -210,6 +212,7 @@ private SearchRequest(
         this.absoluteStartMillis = absoluteStartMillis;
         this.finalReduce = finalReduce;
         this.cancelAfterTimeInterval = searchRequest.cancelAfterTimeInterval;
+        this.phaseTook = searchRequest.phaseTook;
     }
 
     /**
@@ -263,6 +266,9 @@ public SearchRequest(StreamInput in) throws IOException {
         if (in.getVersion().onOrAfter(Version.V_2_7_0)) {
             pipeline = in.readOptionalString();
         }
+        if (in.getVersion().onOrAfter(Version.V_2_12_0)) {
+            phaseTook = in.readOptionalBoolean();
+        }
     }
 
     @Override
@@ -303,6 +309,9 @@ public void writeTo(StreamOutput out) throws IOException {
         if (out.getVersion().onOrAfter(Version.V_2_7_0)) {
             out.writeOptionalString(pipeline);
         }
+        if (out.getVersion().onOrAfter(Version.V_2_12_0)) {
+            out.writeOptionalBoolean(phaseTook);
+        }
     }
 
     @Override
@@ -634,6 +643,20 @@ public void setPreFilterShardSize(int preFilterShardSize) {
         this.preFilterShardSize = preFilterShardSize;
     }
 
+    /**
+     * Returns value of user-provided phase_took query parameter for this search request.
+     */
+    public Boolean isPhaseTook() {
+        return phaseTook;
+    }
+
+    /**
+     * Sets value of phase_took query param if provided by user. Defaults to <code>null</code>.
+     */
+    public void setPhaseTook(Boolean phaseTook) {
+        this.phaseTook = phaseTook;
+    }
+
     /**
      * Returns a threshold that enforces a pre-filter roundtrip to pre-filter search shards based on query rewriting if the number of shards
      * the search request expands to exceeds the threshold, or <code>null</code> if the threshold is unspecified.
@@ -738,7 +761,8 @@ public boolean equals(Object o) {
             && absoluteStartMillis == that.absoluteStartMillis
             && ccsMinimizeRoundtrips == that.ccsMinimizeRoundtrips
             && Objects.equals(cancelAfterTimeInterval, that.cancelAfterTimeInterval)
-            && Objects.equals(pipeline, that.pipeline);
+            && Objects.equals(pipeline, that.pipeline)
+            && Objects.equals(phaseTook, that.phaseTook);
     }
 
     @Override
@@ -759,7 +783,8 @@ public int hashCode() {
             localClusterAlias,
             absoluteStartMillis,
             ccsMinimizeRoundtrips,
-            cancelAfterTimeInterval
+            cancelAfterTimeInterval,
+            phaseTook
         );
     }
 
@@ -802,6 +827,8 @@ public String toString() {
             + cancelAfterTimeInterval
             + ", pipeline="
             + pipeline
+            + ", phaseTook="
+            + phaseTook
             + "}";
     }
 }
diff --git a/server/src/main/java/org/opensearch/action/search/SearchResponse.java b/server/src/main/java/org/opensearch/action/search/SearchResponse.java
@@ -34,6 +34,7 @@
 
 import org.apache.lucene.search.TotalHits;
 import org.opensearch.LegacyESVersion;
+import org.opensearch.Version;
 import org.opensearch.common.Nullable;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.xcontent.StatusToXContentObject;
@@ -64,7 +65,9 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Supplier;
@@ -96,6 +99,7 @@ public class SearchResponse extends ActionResponse implements StatusToXContentOb
     private final ShardSearchFailure[] shardFailures;
     private final Clusters clusters;
     private final long tookInMillis;
+    private final PhaseTook phaseTook;
 
     private List<SearchExtBuilder> searchExtBuilders = new ArrayList<>();
 
@@ -116,6 +120,11 @@ public SearchResponse(StreamInput in) throws IOException {
         clusters = new Clusters(in);
         scrollId = in.readOptionalString();
         tookInMillis = in.readVLong();
+        if (in.getVersion().onOrAfter(Version.V_2_12_0)) {
+            phaseTook = in.readOptionalWriteable(PhaseTook::new);
+        } else {
+            phaseTook = null;
+        }
         skippedShards = in.readVInt();
         if (in.getVersion().onOrAfter(LegacyESVersion.V_7_10_0)) {
             pointInTimeId = in.readOptionalString();
@@ -134,7 +143,32 @@ public SearchResponse(
         ShardSearchFailure[] shardFailures,
         Clusters clusters
     ) {
-        this(internalResponse, scrollId, totalShards, successfulShards, skippedShards, tookInMillis, shardFailures, clusters, null);
+        this(internalResponse, scrollId, totalShards, successfulShards, skippedShards, tookInMillis, null, shardFailures, clusters, null);
+    }
+
+    public SearchResponse(
+        SearchResponseSections internalResponse,
+        String scrollId,
+        int totalShards,
+        int successfulShards,
+        int skippedShards,
+        long tookInMillis,
+        ShardSearchFailure[] shardFailures,
+        Clusters clusters,
+        String pointInTimeId
+    ) {
+        this(
+            internalResponse,
+            scrollId,
+            totalShards,
+            successfulShards,
+            skippedShards,
+            tookInMillis,
+            null,
+            shardFailures,
+            clusters,
+            pointInTimeId
+        );
     }
 
     public SearchResponse(
@@ -144,6 +178,7 @@ public SearchResponse(
         int successfulShards,
         int skippedShards,
         long tookInMillis,
+        PhaseTook phaseTook,
         ShardSearchFailure[] shardFailures,
         Clusters clusters,
         String pointInTimeId
@@ -156,6 +191,7 @@ public SearchResponse(
         this.successfulShards = successfulShards;
         this.skippedShards = skippedShards;
         this.tookInMillis = tookInMillis;
+        this.phaseTook = phaseTook;
         this.shardFailures = shardFailures;
         assert skippedShards <= totalShards : "skipped: " + skippedShards + " total: " + totalShards;
         assert scrollId == null || pointInTimeId == null : "SearchResponse can't have both scrollId ["
@@ -218,6 +254,13 @@ public TimeValue getTook() {
         return new TimeValue(tookInMillis);
     }
 
+    /**
+     * How long the request took in each search phase.
+     */
+    public PhaseTook getPhaseTook() {
+        return phaseTook;
+    }
+
     /**
      * The total number of shards the search was executed on.
      */
@@ -306,6 +349,9 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) t
             builder.field(POINT_IN_TIME_ID.getPreferredName(), pointInTimeId);
         }
         builder.field(TOOK.getPreferredName(), tookInMillis);
+        if (phaseTook != null) {
+            phaseTook.toXContent(builder, params);
+        }
         builder.field(TIMED_OUT.getPreferredName(), isTimedOut());
         if (isTerminatedEarly() != null) {
             builder.field(TERMINATED_EARLY.getPreferredName(), isTerminatedEarly());
@@ -345,6 +391,7 @@ public static SearchResponse innerFromXContent(XContentParser parser) throws IOE
         Boolean terminatedEarly = null;
         int numReducePhases = 1;
         long tookInMillis = -1;
+        PhaseTook phaseTook = null;
         int successfulShards = -1;
         int totalShards = -1;
         int skippedShards = 0; // 0 for BWC
@@ -409,6 +456,24 @@ public static SearchResponse innerFromXContent(XContentParser parser) throws IOE
                             parser.skipChildren();
                         }
                     }
+                } else if (PhaseTook.PHASE_TOOK.match(currentFieldName, parser.getDeprecationHandler())) {
+                    Map<String, Long> phaseTookMap = new HashMap<>();
+
+                    while ((token = parser.nextToken()) != Token.END_OBJECT) {
+                        if (token == Token.FIELD_NAME) {
+                            currentFieldName = parser.currentName();
+                        } else if (token.isValue()) {
+                            try {
+                                SearchPhaseName.valueOf(currentFieldName.toUpperCase(Locale.ROOT));
+                                phaseTookMap.put(currentFieldName, parser.longValue());
+                            } catch (final IllegalArgumentException ex) {
+                                parser.skipChildren();
+                            }
+                        } else {
+                            parser.skipChildren();
+                        }
+                    }
+                    phaseTook = new PhaseTook(phaseTookMap);
                 } else if (Clusters._CLUSTERS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
                     int successful = -1;
                     int total = -1;
@@ -480,6 +545,7 @@ public static SearchResponse innerFromXContent(XContentParser parser) throws IOE
             successfulShards,
             skippedShards,
             tookInMillis,
+            phaseTook,
             failures.toArray(ShardSearchFailure.EMPTY_ARRAY),
             clusters,
             searchContextId
@@ -499,6 +565,9 @@ public void writeTo(StreamOutput out) throws IOException {
         clusters.writeTo(out);
         out.writeOptionalString(scrollId);
         out.writeVLong(tookInMillis);
+        if (out.getVersion().onOrAfter(Version.V_2_12_0)) {
+            out.writeOptionalWriteable(phaseTook);
+        }
         out.writeVInt(skippedShards);
         if (out.getVersion().onOrAfter(LegacyESVersion.V_7_10_0)) {
             out.writeOptionalString(pointInTimeId);
@@ -618,6 +687,67 @@ public String toString() {
         }
     }
 
+    /**
+     * Holds info about the clusters that the search was executed on: how many in total, how many of them were successful
+     * and how many of them were skipped.
+     *
+     * @opensearch.internal
+     */
+    public static class PhaseTook implements ToXContentFragment, Writeable {
+        static final ParseField PHASE_TOOK = new ParseField("phase_took");
+        private final Map<String, Long> phaseTookMap;
+
+        public PhaseTook(Map<String, Long> phaseTookMap) {
+            this.phaseTookMap = phaseTookMap;
+        }
+
+        private PhaseTook(StreamInput in) throws IOException {
+            this(in.readMap(StreamInput::readString, StreamInput::readLong));
+        }
+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            out.writeMap(phaseTookMap, StreamOutput::writeString, StreamOutput::writeLong);
+        }
+
+        @Override
+        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+            builder.startObject(PHASE_TOOK.getPreferredName());
+
+            for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) {
+                if (phaseTookMap.containsKey(searchPhaseName.getName())) {
+                    builder.field(searchPhaseName.getName(), phaseTookMap.get(searchPhaseName.getName()));
+                } else {
+                    builder.field(searchPhaseName.getName(), 0);
+                }
+            }
+            builder.endObject();
+            return builder;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            PhaseTook phaseTook = (PhaseTook) o;
+
+            if (phaseTook.phaseTookMap.equals(phaseTookMap)) {
+                return true;
+            } else {
+                return false;
+            }
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(phaseTookMap);
+        }
+    }
+
     static SearchResponse empty(Supplier<Long> tookInMillisSupplier, Clusters clusters) {
         SearchHits searchHits = new SearchHits(new SearchHit[0], new TotalHits(0L, TotalHits.Relation.EQUAL_TO), Float.NaN);
         InternalSearchResponse internalSearchResponse = new InternalSearchResponse(

diff --git a/server/src/main/java/org/opensearch/action/search/SearchResponseMerger.java b/server/src/main/java/org/opensearch/action/search/SearchResponseMerger.java
@@ -236,6 +236,7 @@ SearchResponse getMergedResponse(SearchResponse.Clusters clusters) {
             successfulShards,
             skippedShards,
             tookInMillis,
+            searchTimeProvider.getPhaseTook(),
             shardFailures,
             clusters,
             null