Add support for Elastic Search _shard_doc equivalent #18924

laminelam · 2025-08-05T16:21:56Z

Implements a new ShardDocSortBuilder and ShardDocFieldComparatorSource to allow sorting by shard id and global doc id as a tie-breaker.

Resolves 17064

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

- Implements ShardDocSortBuilder + comparator - TODO: Add unit + integ tests - Registers in SearchModule Signed-off-by: Lamine Idjeraoui <[email protected]>

Signed-off-by: Lamine Idjeraoui <[email protected]>

github-actions · 2025-08-05T17:44:18Z

❕ Gradle check result for 7199347: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov · 2025-08-05T17:44:58Z

Codecov Report

❌ Patch coverage is 61.17647% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.95%. Comparing base (eb28e77) to head (60f438d).
⚠️ Report is 35 commits behind head on main.

Files with missing lines	Patch %	Lines
...rch/search/sort/ShardDocFieldComparatorSource.java	13.63%	19 Missing ⚠️
...rg/opensearch/search/sort/ShardDocSortBuilder.java	78.04%	7 Missing and 2 partials ⚠️
...va/org/opensearch/action/search/SearchRequest.java	81.25%	1 Missing and 2 partials ⚠️
...nsearch/search/searchafter/SearchAfterBuilder.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #18924      +/-   ##
============================================
+ Coverage     72.81%   72.95%   +0.14%     
- Complexity    69801    69925     +124     
============================================
  Files          5674     5676       +2     
  Lines        320850   320935      +85     
  Branches      46383    46398      +15     
============================================
+ Hits         233638   234153     +515     
+ Misses        68264    67842     -422     
+ Partials      18948    18940       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

LantaoJin · 2025-08-11T08:19:57Z

@laminelam can we add any microbenchmark for it?

github-actions · 2025-09-14T16:06:33Z

❌ Gradle check result for f7d8f73: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

rest-api-spec/src/main/resources/rest-api-spec/test/search/95_search_after_shard_doc.yml

Signed-off-by: Lamine Idjeraoui <[email protected]>

github-actions · 2025-09-15T15:22:30Z

❌ Gradle check result for 9467d06: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

...src/internalClusterTest/java/org/opensearch/search/sort/ShardDocFieldComparatorSourceIT.java

Signed-off-by: Lamine Idjeraoui <[email protected]>

github-actions · 2025-09-21T16:35:32Z

❌ Gradle check result for a719fb7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-09-22T08:08:41Z

❌ Gradle check result for a719fb7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-09-22T10:12:47Z

✅ Gradle check result for a719fb7: SUCCESS

gaobinlong · 2025-09-23T02:17:51Z

Thanks @laminelam, this PR is very close to merge, but seems the code test coverage is low, could you add more some unit tests?

LantaoJin · 2025-09-24T04:05:22Z

Thanks @laminelam , we are preparing the new release version (3.3.0) which the planned code freeze is Sep 30. Do you have a chance to fix the test coverage check ASAP?

laminelam · 2025-09-25T00:59:07Z

Thanks @laminelam , we are preparing the new release version (3.3.0) which the planned code freeze is Sep 30. Do you have a chance to fix the test coverage check ASAP?

Hi @LantaoJin & @gaobinlong
Yes sure, gimme a day or two.
Thx for the review

ivenhov · 2025-09-26T13:43:03Z

Hi
Any change this going to be backported to 2.19 ?

add more test cases Signed-off-by: Lamine Idjeraoui <[email protected]>

github-actions · 2025-09-27T01:01:19Z

❌ Gradle check result for 60f438d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

LantaoJin · 2025-09-28T06:20:44Z

@laminelam, will all PIT search requests add an implicit _shard_doc sort tiebreaker field with this PR?

Example 1:

GET /_search
{
  "size": 10000,
  "query": {
    "match" : {
      "user.id" : "elkbee"
    }
  },
  "pit": {
    "id":  "46ToAwMDaWR5BXV1aWQyKwZub2RlXzMAAAAAAAAAACoBYwADaWR4BXV1aWQxAgZub2RlXzEAAAAAAAAAAAEBYQADaWR5BXV1aWQyKgZub2RlXzIAAAAAAAAAAAwBYgACBXV1aWQyAAAFdXVpZDEAAQltYXRjaF9hbGw_gAAAAA==", 
    "keep_alive": "100m"
  },
  "sort": [ 
    {"@timestamp": {"order": "asc"}}
  ]
}

will sort by @timestamp (asc) + _shard_doc(asc). returns:

{
  "pit_id" : "46ToAwMDaWR5BXV1aWQyKwZub2RlXzMAAAAAAAAAACoBYwADaWR4BXV1aWQxAgZub2RlXzEAAAAAAAAAAAEBYQADaWR5BXV1aWQyKgZub2RlXzIAAAAAAAAAAAwBYgACBXV1aWQyAAAFdXVpZDEAAQltYXRjaF9hbGw_gAAAAA==",
  "took" : 17,
  "timed_out" : false,
  "_shards" : ...,
  "hits" : {
    "total" : ...,
    "max_score" : null,
    "hits" : [
      ...
      {
        "_index" : "my-index-000001",
        "_id" : "FaslK3QBySSL_rrj9zM5",
        "_score" : null,
        "_source" : ...,
        "sort" : [
          "2021-05-20T05:30:04.832Z",
          4294967298
        ]
      }
    ]
  }
}

Example 2

GET /_search
{
  "size": 10000,
  "query": {
    "match" : {
      "user.id" : "elkbee"
    }
  },
  "pit": {
    "id":  "46ToAwMDaWR5BXV1aWQyKwZub2RlXzMAAAAAAAAAACoBYwADaWR4BXV1aWQxAgZub2RlXzEAAAAAAAAAAAEBYQADaWR5BXV1aWQyKgZub2RlXzIAAAAAAAAAAAwBYgACBXV1aWQyAAAFdXVpZDEAAQltYXRjaF9hbGw_gAAAAA==", 
    "keep_alive": "100m"
  },
  "sort": []
}

will sort by _shard_doc(asc), returns

  "pit_id" : "46ToAwMDaWR5BXV1aWQyKwZub2RlXzMAAAAAAAAAACoBYwADaWR4BXV1aWQxAgZub2RlXzEAAAAAAAAAAAEBYQADaWR5BXV1aWQyKgZub2RlXzIAAAAAAAAAAAwBYgACBXV1aWQyAAAFdXVpZDEAAQltYXRjaF9hbGw_gAAAAA==",
  "took" : 17,
  "timed_out" : false,
  "_shards" : ...,
  "hits" : {
    "total" : ...,
    "max_score" : null,
    "hits" : [
      ...
      {
        "_index" : "my-index-000001",
        "_id" : "FaslK3QBySSL_rrj9zM5",
        "_score" : null,
        "_source" : ...,
        "sort" : [
          4294967298
        ]
      }
    ]
  }
}

LantaoJin · 2025-09-28T08:55:59Z

server/src/main/java/org/opensearch/action/search/SearchRequest.java

+        if (sb instanceof FieldSortBuilder) {
+            return ShardDocSortBuilder.NAME.equals(((FieldSortBuilder) sb).getFieldName());
+        }


This logic is never used because we are not allowed to create a FieldSortBuilder("_shard_doc"). I got this exception if I create a FieldSortBuilder("_shard_doc"):

Caused by: org.opensearch.index.query.QueryShardException: No mapping found for [_shard_doc] in order to sort on at org.opensearch.search.sort.FieldSortBuilder.resolveUnmappedType(FieldSortBuilder.java:571) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT] at org.opensearch.search.sort.FieldSortBuilder.build(FieldSortBuilder.java:418) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT] at org.opensearch.search.sort.SortBuilder.buildSort(SortBuilder.java:168) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]

To fix it, we need to update the SearchSourceBuilder.sort(String)

OpenSearch/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java

Lines 590 to 595 in 5e6ee97

public SearchSourceBuilder sort(String name) {

if (name.equals(ScoreSortBuilder.NAME)) {

return sort(SortBuilders.scoreSort());

}

return sort(SortBuilders.fieldSort(name));

}

Hi @LantaoJin
I am not able to replicate the issue.
new FieldSortBuilder(ShardDocSortBuilder.NAME).order(SortOrder.ASC).build(context) is not throwing any exception for me
wondering if we are testing on the same code

This exception was thrown by fcalling SortBuilders.fieldSort("_shard_doc") to build a source builder first then run client.search(request). Not a blocker IMO, I could call SortBuilders.shardDocSort().

Oh ok I get it now. Actually this will not work and it's not the correct way of instantiating a ShardDocSortBuilder, because shard_doc is NOT a real field, it's a kind of pseudo field built on the fly, in the same way score is not.

SortBuilders.fieldSort("fieldName") is for real fields.

There is another method for shard_doc which is SortBuilders.shardDocSort()

In the same you would call scoreSort() for score, scriptSort() for script sort or geoDistanceSort for geoDistance...

LantaoJin · 2025-09-28T09:04:18Z

server/src/test/java/org/opensearch/action/search/SearchRequestTests.java

+            SearchRequest searchRequest = new SearchRequest().source(
+                new SearchSourceBuilder().pointInTimeBuilder(new PointInTimeBuilder("id"))
+                    .sort(new FieldSortBuilder(ShardDocSortBuilder.NAME).order(SortOrder.ASC))
+            );
+            ActionRequestValidationException e = searchRequest.validate();
+            assertNull(e);


Have you try new FieldSortBuilder(ShardDocSortBuilder.NAME).order(SortOrder.ASC).build()? it will fail with

"No mapping found for [" + fieldName + "] in order to sort on"

laminelam · 2025-09-28T15:55:33Z

@laminelam, will all PIT search requests add an implicit _shard_doc sort tiebreaker field with this PR?

Thought do it, actually did it and reverted it back, because I am little bit hesitant to implicitly tie a stable proven feature (PIT) to a newly introduced one (shard_doc sorting) in a single PR. I am thinking we better give the user the option to use PIT without shard_doc and at some point we can make it implicit in a separate PR. I think this is a safest path to avoid breaking anything and also make it backwards compatible.

What are your thoughts?

LantaoJin · 2025-09-29T02:06:28Z

@laminelam, will all PIT search requests add an implicit _shard_doc sort tiebreaker field with this PR?

Thought do it, actually did it and reverted it back, because I am little bit hesitant to implicitly tie a stable proven feature (PIT) to a newly introduced one (shard_doc sorting) in a single PR. I am thinking we better give the user the option to use PIT without shard_doc and at some point we can make it implicit in a separate PR. I think this is a safest path to avoid breaking anything and also make it backwards compatible.

What are your thoughts?

Make sense, LGTM.

gaobinlong · 2025-09-29T13:52:15Z

Hi Any change this going to be backported to 2.19 ?

New feature or enhancement won't be backported to 2.19, only bug fix and security issue fix PR can be backported.

github-actions · 2025-09-29T15:12:01Z

❕ Gradle check result for 60f438d: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Lamine Idjeraoui added 2 commits August 5, 2025 11:08

add a new sort tiebreaker based on shard id and docid

d6edffd

- Implements ShardDocSortBuilder + comparator - TODO: Add unit + integ tests - Registers in SearchModule Signed-off-by: Lamine Idjeraoui <[email protected]>

add a new sort tiebreaker based on shard id and docid

7199347

Signed-off-by: Lamine Idjeraoui <[email protected]>

laminelam requested review from anasalkouz, andrross, ashking94, Bukhtawar, CEHENKLE, cwperks, dbwiddis, gbbafna, jed326, kotwanikunal, mch2, msfroh, owaiskazi19, reta, Rishikesh1159, sachinpkale, saratvemulapalli, shwetathareja, sohami, VachaShah and a team as code owners August 5, 2025 16:21

github-actions bot added enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc labels Aug 6, 2025

This was referenced Aug 11, 2025

[FEATURE] Stabilize preferred SQL implicit sorting method opensearch-project/sql#3064

Open

[FEATURE] Fine-grained implicit sorting opensearch-project/sql#3883

Closed

This was referenced Sep 14, 2025

[AUTOCUT] Gradle Check Flaky Test Report for ClientYamlTestSuiteIT #14319

Open

[AUTOCUT] Gradle Check Flaky Test Report for SmokeTestMultiNodeClientYamlTestSuiteIT #14408

Open

gaobinlong reviewed Sep 15, 2025

View reviewed changes

rest-api-spec/src/main/resources/rest-api-spec/test/search/95_search_after_shard_doc.yml Show resolved Hide resolved

some changes to the shard_doc yaml rest test file

9467d06

Signed-off-by: Lamine Idjeraoui <[email protected]>

gaobinlong reviewed Sep 17, 2025

View reviewed changes

...src/internalClusterTest/java/org/opensearch/search/sort/ShardDocFieldComparatorSourceIT.java Outdated Show resolved Hide resolved

Lamine Idjeraoui added 2 commits September 21, 2025 10:56

Add PIT param to ShardDocFieldComparatorSourceIT's search requests

efa56c9

Signed-off-by: Lamine Idjeraoui <[email protected]>

Merge branch 'main' into feature/shardid_docid_tie_breaker_0508

a719fb7

opensearch-ci-bot mentioned this pull request Sep 21, 2025

[AUTOCUT] Gradle Check Flaky Test Report for IngestFromKinesisIT #17678

Open

opensearch-ci-bot mentioned this pull request Sep 22, 2025

[AUTOCUT] Gradle Check Flaky Test Report for WlmAutoTaggingIT #19354

Open

gaobinlong approved these changes Sep 23, 2025

View reviewed changes

LantaoJin mentioned this pull request Sep 25, 2025

Change the default search sort tiebreaker to _shard_doc for PIT search opensearch-project/sql#4378

Open

8 tasks

add more logic for shard_doc parsing

60f438d

add more test cases Signed-off-by: Lamine Idjeraoui <[email protected]>

opensearch-ci-bot mentioned this pull request Sep 27, 2025

[AUTOCUT] Gradle Check Flaky Test Report for IndexServiceTests #14407

Open

LantaoJin reviewed Sep 28, 2025

View reviewed changes

	public SearchSourceBuilder sort(String name) {
	if (name.equals(ScoreSortBuilder.NAME)) {
	return sort(SortBuilders.scoreSort());
	}
	return sort(SortBuilders.fieldSort(name));
	}

Add support for Elastic Search _shard_doc equivalent #18924

Are you sure you want to change the base?

Add support for Elastic Search _shard_doc equivalent #18924

Conversation

laminelam commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Check List

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

codecov bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LantaoJin commented Aug 11, 2025

Uh oh!

github-actions bot commented Sep 14, 2025

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025

Uh oh!

Uh oh!

github-actions bot commented Sep 21, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

gaobinlong commented Sep 23, 2025

Uh oh!

LantaoJin commented Sep 24, 2025

Uh oh!

laminelam commented Sep 25, 2025

Uh oh!

ivenhov commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 27, 2025

Uh oh!

LantaoJin commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example 1:

Example 2

Uh oh!

LantaoJin Sep 28, 2025

Choose a reason for hiding this comment

Uh oh!

laminelam Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laminelam Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laminelam commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LantaoJin commented Sep 29, 2025

Uh oh!

gaobinlong commented Sep 29, 2025

Uh oh!

github-actions bot commented Sep 29, 2025

Uh oh!

Uh oh!

laminelam commented Aug 5, 2025 •

edited

Loading

codecov bot commented Aug 5, 2025 •

edited

Loading

LantaoJin commented Sep 28, 2025 •

edited

Loading

laminelam Sep 28, 2025 •

edited

Loading

LantaoJin Sep 29, 2025 •

edited

Loading

laminelam Sep 29, 2025 •

edited

Loading

LantaoJin Sep 28, 2025 •

edited

Loading

laminelam commented Sep 28, 2025 •

edited

Loading