Cleaning up sysouts, comments and writing TODOs

Signed-off-by: Vacha Shah <[email protected]>
VachaShah · Nov 18, 2023 · 069828f · 069828f
1 parent fc414ed
commit 069828f
Show file tree

Hide file tree

Showing 26 changed files with 9 additions and 222 deletions.
diff --git a/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/AbstractSearchAsyncAction.java
@@ -194,9 +194,7 @@ long buildTookInMillis() {
      * This is the main entry point for a search. This method starts the search execution of the initial phase.
      */
     public final void start() {
-        System.out.println("AbstractSearchAsyncAction start");
         if (getNumShards() == 0) {
-            System.out.println("Number of shards is 0");
             // no search shards to search on, bail with empty response
             // (it happens with search across _all with no indices around and consistent with broadcast operations)
             int trackTotalHitsUpTo = request.source() == null ? SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO
@@ -224,7 +222,6 @@ public final void start() {
 
     @Override
     public final void run() {
-        System.out.println("AbstractSearchAsyncAction run: running the phase coming from phase.recordAndRun()");
         for (final SearchShardIterator iterator : toSkipShardsIts) {
             assert iterator.skip();
             skipShard(iterator);
@@ -428,8 +425,6 @@ public final void executeNextPhase(SearchPhase currentPhase, SearchPhase nextPha
     }
 
     private void executePhase(SearchPhase phase) {
-        System.out.println("Execute Phase");
-        System.out.println("Phase: " + phase);
         try {
             phase.run();
         } catch (Exception e) {

diff --git a/server/src/main/java/org/opensearch/action/search/ExpandSearchPhase.java b/server/src/main/java/org/opensearch/action/search/ExpandSearchPhase.java
@@ -80,8 +80,6 @@ private boolean isCollapseRequest() {
 
     @Override
     public void run() {
-        System.out.println("ExpandSearchPhase run");
-        System.out.println("Is collapse request: " + isCollapseRequest());
         if (isCollapseRequest() && searchResponse.hits().getHits().length > 0) {
             SearchRequest searchRequest = context.getRequest();
             CollapseBuilder collapseBuilder = searchRequest.source().collapse();

diff --git a/server/src/main/java/org/opensearch/action/search/ProtobufAbstractSearchAsyncAction.java b/server/src/main/java/org/opensearch/action/search/ProtobufAbstractSearchAsyncAction.java
@@ -170,9 +170,7 @@ long buildTookInMillis() {
      * This is the main entry point for a search. This method starts the search execution of the initial phase.
      */
     public final void start() {
-        System.out.println("ProtobufAbstractSearchAsyncAction start");
         if (getNumShards() == 0) {
-            System.out.println("Number of shards is 0");
             // no search shards to search on, bail with empty response
             // (it happens with search across _all with no indices around and consistent with broadcast operations)
             int trackTotalHitsUpTo = request.source() == null ? SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO
@@ -200,7 +198,6 @@ public final void start() {
 
     @Override
     public final void run() {
-        System.out.println("ProtobufAbstractSearchAsyncAction run: running the phase coming from phase.recordAndRun()");
         for (final SearchShardIterator iterator : toSkipShardsIts) {
             assert iterator.skip();
             skipShard(iterator);
@@ -404,8 +401,6 @@ public final void executeNextPhase(SearchPhase currentPhase, SearchPhase nextPha
     }
 
     private void executePhase(SearchPhase phase) {
-        System.out.println("Execute Phase");
-        System.out.println("Phase: " + phase);
         try {
             phase.run();
         } catch (Exception e) {

diff --git a/...r/src/main/java/org/opensearch/action/search/ProtobufSearchQueryThenFetchAsyncAction.java b/...r/src/main/java/org/opensearch/action/search/ProtobufSearchQueryThenFetchAsyncAction.java
@@ -153,7 +153,6 @@ private ProtobufShardSearchRequest rewriteShardSearchRequest(ProtobufShardSearch
         // set the current best bottom field doc
         if (bottomSortCollector.getBottomSortValues() != null) {
             // request.setBottomSortValues(bottomSortCollector.getBottomSortValues());
-            System.out.println("Bottom sort values is not null......now what????");
         }
         return request;
     }

diff --git a/server/src/main/java/org/opensearch/action/search/ProtobufTransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/ProtobufTransportSearchAction.java
@@ -250,7 +250,6 @@ long buildTookInMillis() {
 
     @Override
     protected void doExecute(ProtobufTask task, ProtobufSearchRequest searchRequest, ActionListener<ProtobufSearchResponse> listener) {
-        System.out.println("ProtobufTransportSearchAction doExecute");
         // only if task is of type ProtobufCancellableTask and support cancellation on timeout, treat this request eligible for timeout based
         // cancellation. There may be other top level requests like AsyncSearch which is using ProtobufSearchRequest internally and has it's own
         // cancellation mechanism. For such cases, the ProtobufSearchRequest when created can override the createTask and set the
@@ -361,7 +360,6 @@ private void executeRequest(
         SearchAsyncActionProvider searchAsyncActionProvider,
         ActionListener<ProtobufSearchResponse> originalListener
     ) {
-        System.out.println("TrasportSearchAction executeRequest");
         final long relativeStartNanos = System.nanoTime();
         final SearchTimeProvider timeProvider = new SearchTimeProvider(
             originalSearchRequest.getOrCreateAbsoluteStartMillis(),
@@ -762,8 +760,6 @@ private void executeLocalSearch(
         SearchContextId searchContext,
         SearchAsyncActionProvider searchAsyncActionProvider
     ) {
-        System.out.println("Task is: " + task.getClass().getName());
-        System.out.println("Task is: " + task);
         executeSearch(
             (ProtobufSearchTask) task,
             timeProvider,
@@ -896,7 +892,6 @@ private void executeSearch(
         @Nullable SearchContextId searchContext,
         SearchAsyncActionProvider searchAsyncActionProvider
     ) {
-        System.out.println("ProtobufTransportSearchAction executeSearch");
         clusterState.blocks().globalBlockedRaiseException(ClusterBlockLevel.READ);
 
         // TODO: I think startTime() should become part of ActionRequest and that should be used both for index name
@@ -986,9 +981,6 @@ private void executeSearch(
             concreteLocalIndices,
             localShardIterators.size() + remoteShardIterators.size()
         );
-        System.out.println("Going to ProtobufAbstractSearchAsyncAction");
-        System.out.println("Search request: " + searchRequest);
-        System.out.println("Cluster state: " + clusterState);
         searchAsyncActionProvider.asyncSearchAction(
             task,
             searchRequest,

diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java
@@ -276,7 +276,6 @@ long buildTookInMillis() {
 
     @Override
     protected void doExecute(Task task, SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
-        System.out.println("TransportSearchAction doExecute");
         // only if task is of type CancellableTask and support cancellation on timeout, treat this request eligible for timeout based
         // cancellation. There may be other top level requests like AsyncSearch which is using SearchRequest internally and has it's own
         // cancellation mechanism. For such cases, the SearchRequest when created can override the createTask and set the
@@ -387,7 +386,6 @@ private void executeRequest(
         SearchAsyncActionProvider searchAsyncActionProvider,
         ActionListener<SearchResponse> originalListener
     ) {
-        System.out.println("TrasportSearchAction executeRequest");
         final long relativeStartNanos = System.nanoTime();
         final SearchTimeProvider timeProvider = new SearchTimeProvider(
             originalSearchRequest.getOrCreateAbsoluteStartMillis(),
@@ -788,8 +786,6 @@ private void executeLocalSearch(
         SearchContextId searchContext,
         SearchAsyncActionProvider searchAsyncActionProvider
     ) {
-        System.out.println("Task is: " + task.getClass().getName());
-        System.out.println("Task is: " + task);
         executeSearch(
             (SearchTask) task,
             timeProvider,
@@ -922,7 +918,6 @@ private void executeSearch(
         @Nullable SearchContextId searchContext,
         SearchAsyncActionProvider searchAsyncActionProvider
     ) {
-        System.out.println("TransportSearchAction executeSearch");
         clusterState.blocks().globalBlockedRaiseException(ClusterBlockLevel.READ);
 
         // TODO: I think startTime() should become part of ActionRequest and that should be used both for index name
@@ -1012,9 +1007,6 @@ private void executeSearch(
             concreteLocalIndices,
             localShardIterators.size() + remoteShardIterators.size()
         );
-        System.out.println("Going to AbstractSearchAsyncAction");
-        System.out.println("Search request: " + searchRequest);
-        System.out.println("Cluster state: " + clusterState);
         searchAsyncActionProvider.asyncSearchAction(
             task,
             searchRequest,

diff --git a/server/src/main/java/org/opensearch/action/support/ProtobufTransportAction.java b/server/src/main/java/org/opensearch/action/support/ProtobufTransportAction.java
@@ -72,7 +72,6 @@ public final ProtobufTask execute(Request request, ActionListener<Response> list
 
         try {
             task = taskManager.registerProtobuf("transport", actionName, request);
-            System.out.println("Protobuf task registered from execute is " + task);
         } catch (TaskCancelledException e) {
             unregisterChildNode.close();
             throw e;

diff --git a/server/src/main/java/org/opensearch/action/support/TransportAction.java b/server/src/main/java/org/opensearch/action/support/TransportAction.java
@@ -95,10 +95,8 @@ public final Task execute(Request request, ActionListener<Response> listener) {
         final Releasable unregisterChildNode = registerChildNode(request.getParentTask());
         final Task task;
 
-        System.out.println("TransportAction execute");
         try {
             task = taskManager.register("transport", actionName, request);
-            System.out.println("Task registered from execute is " + task);
         } catch (TaskCancelledException e) {
             unregisterChildNode.close();
             throw e;
@@ -212,7 +210,6 @@ private RequestFilterChain(TransportAction<Request, Response> action, Logger log
 
         @Override
         public void proceed(Task task, String actionName, Request request, ActionListener<Response> listener) {
-            System.out.println("RequestFilterChain proceed");
             int i = index.getAndIncrement();
             try {
                 if (i < this.action.filters.length) {

diff --git a/server/src/main/java/org/opensearch/client/node/NodeClient.java b/server/src/main/java/org/opensearch/client/node/NodeClient.java
@@ -107,7 +107,6 @@ public <Request extends ActionRequest, Response extends ActionResponse> Task exe
         Request request,
         ActionListener<Response> listener
     ) {
-        System.out.println("NodeClient executeLocally");
         return transportAction(action).execute(request, listener);
     }
 
@@ -145,7 +144,6 @@ private <Request extends ActionRequest, Response extends ActionResponse> Transpo
         if (transportAction == null) {
             throw new IllegalStateException("failed to find action [" + action + "] to execute");
         }
-        System.out.println("Getting transport action: " + transportAction);
         return transportAction;
     }
 

diff --git a/server/src/main/java/org/opensearch/client/support/AbstractClient.java b/server/src/main/java/org/opensearch/client/support/AbstractClient.java
@@ -473,7 +473,6 @@ public final <Request extends ActionRequest, Response extends ActionResponse> vo
         Request request,
         ActionListener<Response> listener
     ) {
-        System.out.println("Abstract client execute");
         doExecute(action, request, listener);
     }
 

diff --git a/server/src/main/java/org/opensearch/index/query/Rewriteable.java b/server/src/main/java/org/opensearch/index/query/Rewriteable.java
@@ -112,11 +112,9 @@ static <T extends Rewriteable<T>> void rewriteAndFetch(
         ActionListener<T> rewriteResponse,
         int iteration
     ) {
-        System.out.println("In rewriteAndFetch");
-        System.out.println("Original: " + original.getClass());
-        System.out.println("Context: " + context.getClass());
         T builder = original;
         try {
+            // TODO: add this logic back once it works for proto messages
             // for (T rewrittenBuilder = builder.rewrite(context); rewrittenBuilder != builder; rewrittenBuilder = builder.rewrite(context)) {
             //     builder = rewrittenBuilder;
             //     if (iteration++ >= MAX_REWRITE_ROUNDS) {

diff --git a/server/src/main/java/org/opensearch/rest/action/ProtobufRestCancellableNodeClient.java b/server/src/main/java/org/opensearch/rest/action/ProtobufRestCancellableNodeClient.java
@@ -78,10 +78,6 @@ public <Request extends ProtobufActionRequest, Response extends ProtobufActionRe
         Request request,
         ActionListener<Response> listener
     ) {
-        System.out.println("RestCancellableNodeClient execute");
-        System.out.println("Action: " + action);
-        System.out.println("Request: " + request);
-        System.out.println("Listener: " + listener);
         CloseListener closeListener = httpChannels.computeIfAbsent(httpChannel, channel -> new CloseListener());
         TaskHolder taskHolder = new TaskHolder();
         ProtobufTask task = client.executeLocally(action, request, new ActionListener<Response>() {

diff --git a/server/src/main/java/org/opensearch/rest/action/RestCancellableNodeClient.java b/server/src/main/java/org/opensearch/rest/action/RestCancellableNodeClient.java
@@ -101,10 +101,6 @@ public <Request extends ActionRequest, Response extends ActionResponse> void doE
         Request request,
         ActionListener<Response> listener
     ) {
-        System.out.println("RestCancellableNodeClient execute");
-        System.out.println("Action: " + action);
-        System.out.println("Request: " + request);
-        System.out.println("Listener: " + listener);
         CloseListener closeListener = httpChannels.computeIfAbsent(httpChannel, channel -> new CloseListener());
         TaskHolder taskHolder = new TaskHolder();
         Task task = client.executeLocally(action, request, new ActionListener<Response>() {

diff --git a/server/src/main/java/org/opensearch/rest/action/search/ProtobufRestSearchAction.java b/server/src/main/java/org/opensearch/rest/action/search/ProtobufRestSearchAction.java
@@ -90,7 +90,6 @@ public List<Route> routes() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final ProtobufNodeClient client) throws IOException {
-        System.out.println("In protobuf search API");
         ProtobufSearchRequest searchRequest = new ProtobufSearchRequest();
         /*
          * We have to pull out the call to `source().size(size)` because
@@ -111,7 +110,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final Proto
 
         return channel -> {
             ProtobufRestCancellableNodeClient cancelClient = new ProtobufRestCancellableNodeClient(client, request.getHttpChannel());
-            System.out.println("Cancel client execute");
             cancelClient.execute(ProtobufSearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel));
         };
     }
@@ -193,7 +191,6 @@ public static void parseSearchRequest(
         }
 
         searchRequest.setCancelAfterTimeInterval(request.paramAsTime("cancel_after_time_interval", null));
-        System.out.println("Search request is: " + searchRequest.toString());
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java
@@ -112,7 +112,6 @@ public List<Route> routes() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-        System.out.println("In search API");
         SearchRequest searchRequest = new SearchRequest();
         /*
          * We have to pull out the call to `source().size(size)` because
@@ -133,7 +132,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
 
         return channel -> {
             RestCancellableNodeClient cancelClient = new RestCancellableNodeClient(client, request.getHttpChannel());
-            System.out.println("Cancel client execute");
             cancelClient.execute(SearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel));
         };
     }
@@ -215,7 +213,6 @@ public static void parseSearchRequest(
         }
 
         searchRequest.setCancelAfterTimeInterval(request.paramAsTime("cancel_after_time_interval", null));
-        System.out.println("Search request is: " + searchRequest.toString());
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java
@@ -624,7 +624,6 @@ public void onFailure(Exception exc) {
     }
 
     private IndexShard getShard(ShardSearchRequest request) {
-        System.out.println("getShard");
         if (request.readerId() != null) {
             return findReaderContext(request.readerId(), request).indexShard();
         } else {
@@ -633,7 +632,6 @@ private IndexShard getShard(ShardSearchRequest request) {
     }
 
     private IndexShard getShardProtobuf(ProtobufShardSearchRequest request) {
-        System.out.println("getShardProtobuf");
         if (request.readerId() != null) {
             return findReaderContext(request.readerId(), request).indexShard();
         } else {
@@ -953,17 +951,13 @@ public void executeFetchPhaseProtobuf(ProtobufShardFetchRequest request, Protobu
     }
 
     private ReaderContext getReaderContext(ShardSearchContextId id) {
-        System.out.println("getReaderContext");
-        System.out.println(id.getSessionId());
         if (sessionId.equals(id.getSessionId()) == false && id.getSessionId().isEmpty() == false) {
             throw new SearchContextMissingException(id);
         }
         return activeReaders.get(id.getId());
     }
 
     private ReaderContext findReaderContext(ShardSearchContextId id, TransportRequest request) throws SearchContextMissingException {
-        System.out.println("findReaderContext");
-        System.out.println(id);
         final ReaderContext reader = getReaderContext(id);
         if (reader == null) {
             throw new SearchContextMissingException(id);
@@ -2009,8 +2003,6 @@ public static boolean canRewriteToMatchNone(SearchSourceBuilder source) {
     }
 
     private void rewriteAndFetchShardRequest(IndexShard shard, ShardSearchRequest request, ActionListener<ShardSearchRequest> listener) {
-        System.out.println("SearchService rewriteAndFetchShardRequest");
-        System.out.println("ShardSearchRequest: " + request);
         ActionListener<Rewriteable> actionListener = ActionListener.wrap(r -> {
             if (request.readerId() != null) {
                 listener.onResponse(request);
@@ -2026,8 +2018,6 @@ private void rewriteAndFetchShardRequest(IndexShard shard, ShardSearchRequest re
     }
 
     private void rewriteAndFetchShardRequestProtobuf(IndexShard shard, ProtobufShardSearchRequest request, ActionListener<ProtobufShardSearchRequest> listener) {
-        System.out.println("SearchService rewriteAndFetchShardRequestProtobuf");
-        System.out.println("ProtobufShardSearchRequest: " + request);
         ActionListener<Rewriteable> actionListener = ActionListener.wrap(r -> {
             if (request.readerId() != null) {
                 listener.onResponse(request);

diff --git a/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java b/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java
@@ -55,7 +55,7 @@
  */
 public final class FetchSearchResult extends SearchPhaseResult {
 
-    // TODO: proto message
+    // TODO: Write SearchHits as a proto message
     private SearchHits hits;
     // client side counter
     private transient int counter;

diff --git a/server/src/main/java/org/opensearch/search/fetch/QueryFetchSearchResult.java b/server/src/main/java/org/opensearch/search/fetch/QueryFetchSearchResult.java
@@ -50,7 +50,6 @@
  */
 public final class QueryFetchSearchResult extends SearchPhaseResult {
 
-    // TODO: proto message
     private final QuerySearchResult queryResult;
     private final FetchSearchResult fetchResult;
 
@@ -72,8 +71,6 @@ public QueryFetchSearchResult(byte[] in) throws IOException {
     public QueryFetchSearchResult(QuerySearchResult queryResult, FetchSearchResult fetchResult) {
         this.queryResult = queryResult;
         this.fetchResult = fetchResult;
-        System.out.println("QueryResult: " + queryResult);
-        System.out.println("FetchResult: " + fetchResult);
         if (queryResult.response() != null && fetchResult.response() != null) {
             this.queryFetchSearchResult = QueryFetchSearchResultProto.QueryFetchSearchResult.newBuilder()
                 .setQueryResult(queryResult.response())