diff --git a/server/src/main/java/org/opensearch/action/search/SearchPhaseContext.java b/server/src/main/java/org/opensearch/action/search/SearchPhaseContext.java index 0fa8569413eaf..df451e0745e3c 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchPhaseContext.java +++ b/server/src/main/java/org/opensearch/action/search/SearchPhaseContext.java @@ -34,7 +34,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.OriginalIndices; import org.opensearch.common.Nullable; -import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.common.lease.Releasable; import org.opensearch.common.util.concurrent.AtomicArray; import org.opensearch.search.SearchPhaseResult; @@ -49,9 +49,9 @@ /** * This class provide contextual state and access to resources across multiple search phases. * - * @opensearch.api + * @opensearch.internal */ -@PublicApi(since = "1.0.0") +@InternalApi public interface SearchPhaseContext extends Executor { // TODO maybe we can make this concrete later - for now we just implement this in the base class for all initial phases diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java index ecf48b4a9f633..c81061514e873 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java @@ -10,23 +10,23 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.annotation.InternalApi; import java.util.List; /** * A listener for search, fetch and context events at the coordinator node level * - * @opensearch.api + * @opensearch.internal */ -@PublicApi(since = "1.0.0") -public interface SearchRequestOperationsListener { +@InternalApi +interface SearchRequestOperationsListener { - void onPhaseStart(SearchPhaseContext context); + void onPhaseStart(SearchPhase phase); - void onPhaseEnd(SearchPhaseContext context); + void onPhaseEnd(SearchPhase phase); - void onPhaseFailure(SearchPhaseContext context); + void onPhaseFailure(SearchPhase phase); /** * Holder of Composite Listeners @@ -44,10 +44,10 @@ public CompositeListener(List listeners, Logger } @Override - public void onPhaseStart(SearchPhaseContext context) { + public void onPhaseStart(SearchPhase phase) { for (SearchRequestOperationsListener listener : listeners) { try { - listener.onPhaseStart(context); + listener.onPhaseStart(phase); } catch (Exception e) { logger.warn(() -> new ParameterizedMessage("onPhaseStart listener [{}] failed", listener), e); } @@ -55,10 +55,10 @@ public void onPhaseStart(SearchPhaseContext context) { } @Override - public void onPhaseEnd(SearchPhaseContext context) { + public void onPhaseEnd(SearchPhase phase) { for (SearchRequestOperationsListener listener : listeners) { try { - listener.onPhaseEnd(context); + listener.onPhaseEnd(phase); } catch (Exception e) { logger.warn(() -> new ParameterizedMessage("onPhaseEnd listener [{}] failed", listener), e); } @@ -66,10 +66,10 @@ public void onPhaseEnd(SearchPhaseContext context) { } @Override - public void onPhaseFailure(SearchPhaseContext context) { + public void onPhaseFailure(SearchPhase phase) { for (SearchRequestOperationsListener listener : listeners) { try { - listener.onPhaseFailure(context); + listener.onPhaseFailure(phase); } catch (Exception e) { logger.warn(() -> new ParameterizedMessage("onPhaseFailure listener [{}] failed", listener), e); } diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java index 6b7c94ec3037a..d791b21a9d18b 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java @@ -46,21 +46,21 @@ public long getPhaseMetric(SearchPhaseName searchPhaseName) { } @Override - public void onPhaseStart(SearchPhaseContext context) { - phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.inc(); + public void onPhaseStart(SearchPhase phase) { + phaseStatsMap.get(phase.getSearchPhaseName()).current.inc(); } @Override - public void onPhaseEnd(SearchPhaseContext context) { - StatsHolder phaseStats = phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()); + public void onPhaseEnd(SearchPhase phase) { + StatsHolder phaseStats = phaseStatsMap.get(phase.getSearchPhaseName()); phaseStats.current.dec(); phaseStats.total.inc(); - phaseStats.timing.inc(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - context.getCurrentPhase().getStartTimeInNanos())); + phaseStats.timing.inc(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - phase.getStartTimeInNanos())); } @Override - public void onPhaseFailure(SearchPhaseContext context) { - phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec(); + public void onPhaseFailure(SearchPhase phase) { + phaseStatsMap.get(phase.getSearchPhaseName()).current.dec(); } /** diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index 16b7e4810b130..4d8519e7393b8 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -354,18 +354,15 @@ SearchResponse.PhaseTook getPhaseTook() { Map phaseStatsMap = new EnumMap<>(SearchPhaseName.class); @Override - public void onPhaseStart(SearchPhaseContext context) {} + public void onPhaseStart(SearchPhase phase) {} @Override - public void onPhaseEnd(SearchPhaseContext context) { - phaseStatsMap.put( - context.getCurrentPhase().getSearchPhaseName(), - TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - context.getCurrentPhase().getStartTimeInNanos()) - ); + public void onPhaseEnd(SearchPhase phase) { + phaseStatsMap.put(phase.getSearchPhaseName(), TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - phase.getStartTimeInNanos())); } @Override - public void onPhaseFailure(SearchPhaseContext context) {} + public void onPhaseFailure(SearchPhase phase) {} public Long getPhaseTookTime(SearchPhaseName searchPhaseName) { return phaseStatsMap.get(searchPhaseName); diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerTests.java index ef880043e863c..fb1fd6c34b8f5 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerTests.java @@ -29,19 +29,19 @@ public void testListenersAreExecuted() { SearchRequestOperationsListener testListener = new SearchRequestOperationsListener() { @Override - public void onPhaseStart(SearchPhaseContext context) { - searchPhaseMap.get(context.getCurrentPhase().getSearchPhaseName()).current.inc(); + public void onPhaseStart(SearchPhase phase) { + searchPhaseMap.get(phase.getSearchPhaseName()).current.inc(); } @Override - public void onPhaseEnd(SearchPhaseContext context) { - searchPhaseMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec(); - searchPhaseMap.get(context.getCurrentPhase().getSearchPhaseName()).total.inc(); + public void onPhaseEnd(SearchPhase phase) { + searchPhaseMap.get(phase.getSearchPhaseName()).current.dec(); + searchPhaseMap.get(phase.getSearchPhaseName()).total.inc(); } @Override - public void onPhaseFailure(SearchPhaseContext context) { - searchPhaseMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec(); + public void onPhaseFailure(SearchPhase phase) { + searchPhaseMap.get(phase.getSearchPhaseName()).current.dec(); } }; @@ -62,7 +62,7 @@ public void onPhaseFailure(SearchPhaseContext context) { for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) { when(ctx.getCurrentPhase()).thenReturn(searchPhase); when(searchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); - compositeListener.onPhaseStart(ctx); + compositeListener.onPhaseStart(ctx.getCurrentPhase()); assertEquals(totalListeners, searchPhaseMap.get(searchPhaseName).current.count()); } } diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestStatsTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestStatsTests.java index f24147a8194b4..c5fbff144dff9 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestStatsTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestStatsTests.java @@ -29,9 +29,9 @@ public void testSearchRequestPhaseFailure() { for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) { when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); - testRequestStats.onPhaseStart(ctx); + testRequestStats.onPhaseStart(ctx.getCurrentPhase()); assertEquals(1, testRequestStats.getPhaseCurrent(searchPhaseName)); - testRequestStats.onPhaseFailure(ctx); + testRequestStats.onPhaseFailure(ctx.getCurrentPhase()); assertEquals(0, testRequestStats.getPhaseCurrent(searchPhaseName)); } } @@ -46,11 +46,11 @@ public void testSearchRequestStats() { for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) { when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); long tookTimeInMillis = randomIntBetween(1, 10); - testRequestStats.onPhaseStart(ctx); + testRequestStats.onPhaseStart(ctx.getCurrentPhase()); long startTime = System.nanoTime() - TimeUnit.MILLISECONDS.toNanos(tookTimeInMillis); when(mockSearchPhase.getStartTimeInNanos()).thenReturn(startTime); assertEquals(1, testRequestStats.getPhaseCurrent(searchPhaseName)); - testRequestStats.onPhaseEnd(ctx); + testRequestStats.onPhaseEnd(ctx.getCurrentPhase()); assertEquals(0, testRequestStats.getPhaseCurrent(searchPhaseName)); assertEquals(1, testRequestStats.getPhaseTotal(searchPhaseName)); assertThat(testRequestStats.getPhaseMetric(searchPhaseName), greaterThanOrEqualTo(tookTimeInMillis)); @@ -71,7 +71,7 @@ public void testSearchRequestStatsOnPhaseStartConcurrently() throws InterruptedE for (int i = 0; i < numTasks; i++) { threads[i] = new Thread(() -> { phaser.arriveAndAwaitAdvance(); - testRequestStats.onPhaseStart(ctx); + testRequestStats.onPhaseStart(ctx.getCurrentPhase()); countDownLatch.countDown(); }); threads[i].start(); @@ -102,7 +102,7 @@ public void testSearchRequestStatsOnPhaseEndConcurrently() throws InterruptedExc for (int i = 0; i < numTasks; i++) { threads[i] = new Thread(() -> { phaser.arriveAndAwaitAdvance(); - testRequestStats.onPhaseEnd(ctx); + testRequestStats.onPhaseEnd(ctx.getCurrentPhase()); countDownLatch.countDown(); }); threads[i].start(); @@ -134,8 +134,8 @@ public void testSearchRequestStatsOnPhaseFailureConcurrently() throws Interrupte for (int i = 0; i < numTasks; i++) { threads[i] = new Thread(() -> { phaser.arriveAndAwaitAdvance(); - testRequestStats.onPhaseStart(ctx); - testRequestStats.onPhaseFailure(ctx); + testRequestStats.onPhaseStart(ctx.getCurrentPhase()); + testRequestStats.onPhaseFailure(ctx.getCurrentPhase()); countDownLatch.countDown(); }); threads[i].start(); diff --git a/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java b/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java index f0f1a43e6c21e..e229effc44c9a 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java @@ -26,9 +26,9 @@ public void testSearchTimeProviderPhaseFailure() { for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) { when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); - testTimeProvider.onPhaseStart(ctx); + testTimeProvider.onPhaseStart(ctx.getCurrentPhase()); assertNull(testTimeProvider.getPhaseTookTime(searchPhaseName)); - testTimeProvider.onPhaseFailure(ctx); + testTimeProvider.onPhaseFailure(ctx.getCurrentPhase()); assertNull(testTimeProvider.getPhaseTookTime(searchPhaseName)); } } @@ -43,11 +43,11 @@ public void testSearchTimeProviderPhaseEnd() { for (SearchPhaseName searchPhaseName : SearchPhaseName.values()) { when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); long tookTimeInMillis = randomIntBetween(1, 100); - testTimeProvider.onPhaseStart(ctx); + testTimeProvider.onPhaseStart(ctx.getCurrentPhase()); long startTime = System.nanoTime() - TimeUnit.MILLISECONDS.toNanos(tookTimeInMillis); when(mockSearchPhase.getStartTimeInNanos()).thenReturn(startTime); assertNull(testTimeProvider.getPhaseTookTime(searchPhaseName)); - testTimeProvider.onPhaseEnd(ctx); + testTimeProvider.onPhaseEnd(ctx.getCurrentPhase()); assertThat(testTimeProvider.getPhaseTookTime(searchPhaseName), greaterThanOrEqualTo(tookTimeInMillis)); } } diff --git a/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java b/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java index c27e4bf27327a..79954e2ef928c 100644 --- a/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java +++ b/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java @@ -84,8 +84,8 @@ public void testShardLevelSearchGroupStats() throws Exception { when(mockSearchPhase.getStartTimeInNanos()).thenReturn(System.nanoTime() - TimeUnit.SECONDS.toNanos(paramValue)); when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); for (int iterator = 0; iterator < paramValue; iterator++) { - testRequestStats.onPhaseStart(ctx); - testRequestStats.onPhaseEnd(ctx); + testRequestStats.onPhaseStart(ctx.getCurrentPhase()); + testRequestStats.onPhaseEnd(ctx.getCurrentPhase()); } } searchStats1.setSearchRequestStats(testRequestStats);