MRT should default to true for CPS searches

Author: pawankartik-elastic

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java

@@ -23,6 +23,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 
@@ -37,11 +38,11 @@ public class RestSearchTemplateAction extends BaseRestHandler {
     private static final Set<String> RESPONSE_PARAMS = Set.of(TYPED_KEYS_PARAM, RestSearchAction.TOTAL_HITS_AS_INT_PARAM);
 
     private final Predicate<NodeFeature> clusterSupportsFeature;
-    private final Settings settings;
+    private final boolean crossProjectEnabled;
 
     public RestSearchTemplateAction(Predicate<NodeFeature> clusterSupportsFeature, Settings settings) {
         this.clusterSupportsFeature = clusterSupportsFeature;
-        this.settings = settings;
+        this.crossProjectEnabled = settings != null && settings.getAsBoolean("serverless.cross_project.enabled", false);
     }
 
     @Override
@@ -61,7 +62,7 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
-        if (settings != null && settings.getAsBoolean("serverless.cross_project.enabled", false)) {
+        if (crossProjectEnabled) {
             // accept but drop project_routing param until fully supported
             request.param("project_routing");
         }
@@ -73,7 +74,9 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
             request,
             null,
             clusterSupportsFeature,
-            size -> searchRequest.source().size(size)
+            size -> searchRequest.source().size(size),
+            null,
+            Optional.of(crossProjectEnabled)
         );
 
         // Creates the search template request

server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -44,6 +44,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.IntConsumer;
 import java.util.function.Predicate;
@@ -68,13 +69,11 @@ public class RestSearchAction extends BaseRestHandler {
 
     private final SearchUsageHolder searchUsageHolder;
     private final Predicate<NodeFeature> clusterSupportsFeature;
-    private final Settings settings;
     private final CrossProjectModeDecider crossProjectModeDecider;
 
     public RestSearchAction(SearchUsageHolder searchUsageHolder, Predicate<NodeFeature> clusterSupportsFeature, Settings settings) {
         this.searchUsageHolder = searchUsageHolder;
         this.clusterSupportsFeature = clusterSupportsFeature;
-        this.settings = settings;
         this.crossProjectModeDecider = new CrossProjectModeDecider(settings);
     }
 
@@ -134,7 +133,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
                 clusterSupportsFeature,
                 setSize,
                 searchUsageHolder,
-                crossProjectEnabled
+                Optional.of(crossProjectEnabled)
             )
         );
 
@@ -146,7 +145,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
 
     /**
      * Parses the rest request on top of the SearchRequest, preserving values that are not overridden by the rest request.
-     *
+     * The endpoint calling this method is treated as if it does not support Cross Project Search (CPS). In case it supports
+     * CPS, it should call in the other appropriate overload and pass in the CPS state explicitly either via Optional.of(true)
+     * or Optional.of(false).
      * @param searchRequest the search request that will hold what gets parsed
      * @param request the rest request to read from
      * @param requestContentParser body of the request to read. This method does not attempt to read the body from the {@code request}
@@ -172,7 +173,15 @@ public static void parseSearchRequest(
         IntConsumer setSize,
         @Nullable SearchUsageHolder searchUsageHolder
     ) throws IOException {
-        parseSearchRequest(searchRequest, request, requestContentParser, clusterSupportsFeature, setSize, searchUsageHolder, false);
+        parseSearchRequest(
+            searchRequest,
+            request,
+            requestContentParser,
+            clusterSupportsFeature,
+            setSize,
+            searchUsageHolder,
+            Optional.empty()
+        );
     }
 
     /**
@@ -185,7 +194,10 @@ public static void parseSearchRequest(
      * @param clusterSupportsFeature used to check if certain features are available in this cluster
      * @param setSize how the size url parameter is handled. {@code udpate_by_query} and regular search differ here.
      * @param searchUsageHolder the holder of search usage stats
-     * @param crossProjectEnabled whether serverless.cross_project.enabled is set to true
+     * @param crossProjectEnabled Specifies the state of Cross Project Search (CPS) for the endpoint that's calling this method.
+     *                            Optional.of(true)  - signifies that the endpoint supports CPS,
+     *                            Optional.of(false) - signifies that the endpoint supports CPS but CPS is disabled, and,
+     *                            Optional.empty()   - signifies that the endpoint does not support CPS.
      */
     public static void parseSearchRequest(
         SearchRequest searchRequest,
@@ -194,7 +206,7 @@ public static void parseSearchRequest(
         Predicate<NodeFeature> clusterSupportsFeature,
         IntConsumer setSize,
         @Nullable SearchUsageHolder searchUsageHolder,
-        boolean crossProjectEnabled
+        Optional<Boolean> crossProjectEnabled
     ) throws IOException {
         if (searchRequest.source() == null) {
             searchRequest.source(new SearchSourceBuilder());
@@ -205,7 +217,7 @@ public static void parseSearchRequest(
          * We only do it if in a Cross Project Environment, though, because outside it, such details are not
          * expected and valid.
          */
-        SearchRequest searchRequestForParsing = crossProjectEnabled ? searchRequest : null;
+        SearchRequest searchRequestForParsing = crossProjectEnabled.orElse(false) ? searchRequest : null;
         if (requestContentParser != null) {
             if (searchUsageHolder == null) {
                 searchRequest.source().parseXContent(searchRequestForParsing, requestContentParser, true, clusterSupportsFeature);
@@ -250,7 +262,7 @@ public static void parseSearchRequest(
         searchRequest.routing(request.param("routing"));
         searchRequest.preference(request.param("preference"));
         IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, searchRequest.indicesOptions());
-        if (crossProjectEnabled && searchRequest.allowsCrossProject() && searchRequest.pointInTimeBuilder() == null) {
+        if (crossProjectEnabled.orElse(false) && searchRequest.allowsCrossProject() && searchRequest.pointInTimeBuilder() == null) {
             indicesOptions = IndicesOptions.builder(indicesOptions)
                 .crossProjectModeOptions(new IndicesOptions.CrossProjectModeOptions(true))
                 .build();
@@ -262,9 +274,30 @@ public static void parseSearchRequest(
         if (searchRequest.pointInTimeBuilder() != null) {
             preparePointInTime(searchRequest, request);
         } else {
-            searchRequest.setCcsMinimizeRoundtrips(
-                request.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
-            );
+            if (crossProjectEnabled.orElse(false)) {
+                /*
+                 * MRT should not be settable by the user when in Cross Project Search environment.
+                 * Only _async_search uses MRT=false. However, in RestSubmitAsyncSearchAction, we
+                 * already, explicitly, and directly call in `SearchRequest#setCcsMinimizeRoundtrips()`
+                 * to set it to true. Although other searches that utilise SearchRequest-s do not call
+                 * this method, SearchRequest, by default, sets MRT to true when it is instantiated.
+                 * This way, all searches pivot to MRT=true for CPS.
+                 *
+                 * Since users will anyway see a banner via Kibana that setting MRT in Serverless has no
+                 * effect, we can safely drop it.
+                 */
+                if (request.hasParam("ccs_minimize_roundtrips")) {
+                    request.param("ccs_minimize_roundtrips");
+                }
+            } else {
+                /*
+                 * Either we're not in a Cross Project Search environment or the endpoint isn't compatible with it. Parse what's in the
+                 * request.
+                 */
+                searchRequest.setCcsMinimizeRoundtrips(
+                    request.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
+                );
+            }
         }
         if (request.paramAsBoolean("force_synthetic_source", false)) {
             searchRequest.setForceSyntheticSource(true);

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java

@@ -25,6 +25,7 @@
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.IntConsumer;
 import java.util.function.Predicate;
@@ -70,6 +71,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
 
         boolean crossProjectEnabled = crossProjectModeDecider.crossProjectEnabled();
         if (crossProjectEnabled) {
+            submit.getSearchRequest().setCcsMinimizeRoundtrips(true);
             submit.getSearchRequest().setProjectRouting(request.param("project_routing"));
         }
 
@@ -86,7 +88,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
                 clusterSupportsFeature,
                 setSize,
                 searchUsageHolder,
-                crossProjectEnabled
+                Optional.of(crossProjectEnabled)
             )
         );