Skip to content

Conversation

@pawankartik-elastic
Copy link
Contributor

Here's the before code:

parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex, (searchRequest, parser) -> {
    // ...
    if (searchRequest.pointInTimeBuilder() != null) {
        // ...
    } else {
        searchRequest.setCcsMinimizeRoundtrips( // <-- This is problematic; by now parsing and reading are done!
            SearchParamsParser.parseCcsMinimizeRoundtrips(crossProjectEnabled, restRequest, searchRequest.isCcsMinimizeRoundtrips())
        );
    }
    multiRequest.add(searchRequest);
}, /* ... */ );

parseMultiLineRequest() is responsible for parsing the msearch's body. Within this method, we instantiate a new SearchRequest object for each request and set its MRT to whatever's mentioned in the query parameter (the default fallback value is true). Then, the body of each request is read by readMultiLineFormat(), which extracts the MRT value in the request's body (if present) and sets it by overwriting the previously set value. This way, the MRT value in the request body takes precedence over the query parameter.

However, when the parsing and reading are done, we end up setting the query param value again before adding the request to the msearch object (ref above code and the code comment). Whatever value we read from the request's body is now lost; effectively, we're ignoring whatever value is present in the request's body.

The fix is to drop the problematic MRT setting op after the request's parsing is complete.

@pawankartik-elastic
Copy link
Contributor Author

Will mark it as ready once the CI issue is resolved.

@pawankartik-elastic pawankartik-elastic marked this pull request as ready for review December 10, 2025 17:54
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@elasticsearchmachine
Copy link
Collaborator

Hi @pawankartik-elastic, I've created a changelog YAML for you.

public void testMrtValuesArePickedCorrectly() throws IOException {

{
// If no MRT is specified, all searches should default to true.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it default to true?
Is that because _msearch always uses _search rather than _async_search and _search defaults to true?

Copy link
Contributor Author

@pawankartik-elastic pawankartik-elastic Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. And when the SearchRequest object is instantiated, MRT defaults to true.

Copy link
Contributor

@quux00 quux00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pawankartik-elastic pawankartik-elastic merged commit 3d0a647 into elastic:main Dec 16, 2025
36 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
9.1 Commit could not be cherrypicked due to conflicts
9.2 Commit could not be cherrypicked due to conflicts
8.19 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 138583

pawankartik-elastic added a commit to pawankartik-elastic/elasticsearch that referenced this pull request Dec 16, 2025
…tic#138583)

Do not override MRT from the request's body with the query
param value.

(cherry picked from commit 3d0a647)

# Conflicts:
#	server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java
@pawankartik-elastic
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
9.2
9.1
8.19

Questions ?

Please refer to the Backport tool documentation

pawankartik-elastic added a commit to pawankartik-elastic/elasticsearch that referenced this pull request Dec 16, 2025
…tic#138583)

Do not override MRT from the request's body with the query
param value.

(cherry picked from commit 3d0a647)

# Conflicts:
#	server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java
pawankartik-elastic added a commit to pawankartik-elastic/elasticsearch that referenced this pull request Dec 16, 2025
…tic#138583)

Do not override MRT from the request's body with the query
param value.

(cherry picked from commit 3d0a647)

# Conflicts:
#	server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java
elasticsearchmachine pushed a commit that referenced this pull request Dec 16, 2025
#138583) (#139586)

* fix: Correctly pickup MRT value for `msearch`'s search requests (#138583)

Do not override MRT from the request's body with the query
param value.

(cherry picked from commit 3d0a647)

# Conflicts:
#	server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java

* Fix CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >bug :Search Foundations/CCS Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.19.10 v9.1.10 v9.2.4 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants