Addressing comments and adding CHANGELOG

Signed-off-by: Sagar Upadhyaya <[email protected]>
buddharajusahil · Sep 11, 2023 · 9e7baaa · 9e7baaa
1 parent afc4a35
commit 9e7baaa
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -89,6 +89,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Segment Replication] Support realtime reads for GET requests ([#9212](https://github.com/opensearch-project/OpenSearch/pull/9212))
 - [Feature] Expose term frequency in Painless script score context ([#9081](https://github.com/opensearch-project/OpenSearch/pull/9081))
 - Add support for reading partial files to HDFS repository ([#9513](https://github.com/opensearch-project/OpenSearch/issues/9513))
+- Add coordinator level stats for search latency ([#8386](https://github.com/opensearch-project/OpenSearch/issues/8386))
 
 ### Dependencies
 - Bump `org.apache.logging.log4j:log4j-core` from 2.17.1 to 2.20.0 ([#8307](https://github.com/opensearch-project/OpenSearch/pull/8307))

diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java
@@ -561,7 +561,7 @@ private void onPhaseEnd(SearchPhaseContext searchPhaseContext) {
     private void onPhaseStart(SearchPhase phase, SearchPhaseContext searchPhaseContext) {
         setCurrentPhase(phase);
         phase.setStartTimeInNanos(System.nanoTime());
-        if (!(searchListenersList == null) && !searchListenersList.isEmpty()) {
+        if (!CollectionUtils.isEmpty(searchListenersList)) {
             if (searchPhaseContext.getCurrentPhase() != null
                 && searchPhaseStartTrackingMap.containsKey(searchPhaseContext.getCurrentPhase().getName())) {
                 searchPhaseStartTrackingMap.get(searchPhaseContext.getCurrentPhase().getName()).run();
@@ -831,7 +831,7 @@ public void sendSearchResponse(InternalSearchResponse internalSearchResponse, At
 
     @Override
     public final void onPhaseFailure(SearchPhase phase, String msg, Throwable cause) {
-        if (!(searchListenersList == null) && !searchListenersList.isEmpty()) {
+        if (!CollectionUtils.isEmpty(searchListenersList)) {
             if (this.getCurrentPhase() != null && searchPhaseFailureTrackingMap.containsKey(this.getCurrentPhase().getName())) {
                 searchPhaseFailureTrackingMap.get(this.getCurrentPhase().getName()).run();
             }

diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java
@@ -350,7 +350,7 @@ public AbstractSearchAsyncAction<? extends SearchPhaseResult> asyncSearchAction(
                 ThreadPool threadPool,
                 SearchResponse.Clusters clusters
             ) {
-                AbstractSearchAsyncAction<SearchPhaseResult> returnAbstractSearchAsyncAction = new AbstractSearchAsyncAction<
+                return new AbstractSearchAsyncAction<
                     SearchPhaseResult>(
                         actionName,
                         logger,
@@ -397,7 +397,6 @@ boolean buildPointInTimeFromSearchResults() {
                         return includeSearchContext;
                     }
                 };
-                return returnAbstractSearchAsyncAction;
             }
         }, listener);
     }

diff --git a/server/src/main/java/org/opensearch/index/search/stats/SearchStats.java b/server/src/main/java/org/opensearch/index/search/stats/SearchStats.java
@@ -110,12 +110,9 @@ public static class Stats implements Writeable, ToXContentFragment {
         private long pitCurrent;
 
         @Nullable
-        public RequestStatsLongHolder requestStatsLongHolder;
+        private RequestStatsLongHolder requestStatsLongHolder;
 
         public RequestStatsLongHolder getRequestStatsLongHolder() {
-            if (requestStatsLongHolder == null) {
-                return null;
-            }
             return requestStatsLongHolder;
         }
 
@@ -359,7 +356,10 @@ public void writeTo(StreamOutput out) throws IOException {
                 out.writeVLong(pitCurrent);
             }
 
-            if (out.getVersion().onOrAfter(Version.V_3_0_0) && requestStatsLongHolder != null) {
+            if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
+                if (requestStatsLongHolder == null) {
+                    requestStatsLongHolder = new RequestStatsLongHolder();
+                }
                 out.writeVLong(requestStatsLongHolder.dfsPreQueryMetric);
                 out.writeVLong(requestStatsLongHolder.dfsPreQueryCurrent);
                 out.writeVLong(requestStatsLongHolder.dfsPreQueryTotal);

diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java
@@ -32,6 +32,7 @@
 
 package org.opensearch.action.admin.cluster.node.stats;
 
+import org.opensearch.action.RequestStats;
 import org.opensearch.action.admin.indices.stats.CommonStats;
 import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
 import org.opensearch.cluster.coordination.PendingClusterStateStats;
@@ -784,7 +785,7 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) {
     private static NodeIndicesStats getNodeIndicesStats(boolean remoteStoreStats) {
         NodeIndicesStats indicesStats = null;
         if (remoteStoreStats) {
-            indicesStats = new NodeIndicesStats(new CommonStats(CommonStatsFlags.ALL), new HashMap<>());
+            indicesStats = new NodeIndicesStats(new CommonStats(CommonStatsFlags.ALL), new HashMap<>(), new RequestStats());
             RemoteSegmentStats remoteSegmentStats = indicesStats.getSegments().getRemoteSegmentStats();
             remoteSegmentStats.addUploadBytesStarted(10L);
             remoteSegmentStats.addUploadBytesSucceeded(10L);