Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle canMatchSearchAfter for frozen context scenario #11249

Merged
merged 4 commits into from
Nov 20, 2023

Conversation

gashutos
Copy link
Contributor

@gashutos gashutos commented Nov 17, 2023

Description

There is a corner case where sort is failing on nested field after this PR #7453. When nested field is date type and have filter in nested field has now() function call.
OpenSearch prevents SortFields' initialization in such cases more then one time.
This is very corner scenario but I added integ tests for that as we didn't have any integ test.
As a mitigation, I am pssing already initialized SortAndFormats in ContextIndexSearcher in this PR. In fact I also made change to invoke that if clause only if SearchContext contains searchAfter.

This happens only when nested_sort is having date field type and filter is having range along with now(). Culprit is the now() call that Opensearch is trying to avoid since now() should get called only once.

How to repro the issue

PUT test_sort_nested_date2
{
  "settings" : {
      "number_of_shards" : 1,
      "number_of_replicas" : 0
  },
  "mappings": {
    "properties": {
      "nested_field": {
        "type": "nested",
        "properties": {
          "date_field": {
            "type": "date",
            "format": "date_optional_time"
          }
        }
      }
    }
  }
}
POST test_sort_nested_date2/_bulk
{"index": {"_id": "1"}}
{"nested_field": [{"date_field": "2023-10-26T12:00:00+09:00"}, {"date_field": "2025-10-01T12:00:00+09:00"}]}
GET test_sort_nested_date2/_search
{
  "sort": [
    {
      "nested_field.date_field": {
      "mode": "max",
        "order": "desc",
        "nested": {
          "path": "nested_field",
          "filter": {
            "bool": {
              "filter": [
                {
                  "range": {
                    "nested_field.date_field": {
                      "gte": "now/h",
                      "time_zone": "+09:00"
                    }
                  }
                }
              ]
            }
          }
        }
      }
    }
  ]
}

This search query failes with below Exception,

{
  "error": {
    "root_cause": [
      {
        "type": "parse_exception",
        "reason": "could not read the current timestamp"
      }
    ],
    "type": "search_phase_execution_exception",
    "reason": "all shards failed",
    "phase": "query",
    "grouped": true,
    "failed_shards": [
      {
        "shard": 0,
        "index": "test_sort_nested_date2",
        "node": "WT1VH4gyTVGVZyi_tNMO-Q",
        "reason": {
          "type": "parse_exception",
          "reason": "could not read the current timestamp",
          "caused_by": {
            "type": "illegal_argument_exception",
            "reason": "features that prevent cachability are disabled on this context"
          }
        }
      }
    ]
  },
  "status": 400
}

Server side traces are corelated with canMatchSearchAfter where we are trying to initialize SortAndFormats again to get MinMax values.

[2023-10-21T12:21:59,877][DEBUG][o.o.a.s.TransportSearchAction] [bc908df47a785e9192f108cf1282c4e4] All shards failed for phase: [query]
OpenSearchParseException[could not read the current timestamp]; nested: IllegalArgumentException[features that prevent cachability are disabled on this context];
    at org.opensearch.common.time.JavaDateMathParser.parse(JavaDateMathParser.java:83)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.parseToLong(DateFieldMapper.java:510)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.lambda$dateRangeQuery$1(DateFieldMapper.java:464)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.handleNow(DateFieldMapper.java:493)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.dateRangeQuery(DateFieldMapper.java:459)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.rangeQuery(DateFieldMapper.java:434)
    at org.opensearch.index.query.RangeQueryBuilder.doToQuery(RangeQueryBuilder.java:527)
    at org.opensearch.index.query.AbstractQueryBuilder.toQuery(AbstractQueryBuilder.java:117)
    at org.opensearch.index.query.BoolQueryBuilder.addBooleanClauses(BoolQueryBuilder.java:346)
    at org.opensearch.index.query.BoolQueryBuilder.doToQuery(BoolQueryBuilder.java:329)
    at org.opensearch.index.query.AbstractQueryBuilder.toQuery(AbstractQueryBuilder.java:117)
    at org.opensearch.search.sort.SortBuilder.resolveNestedQuery(SortBuilder.java:245)
    at org.opensearch.search.sort.SortBuilder.resolveNested(SortBuilder.java:203)
    at org.opensearch.search.sort.FieldSortBuilder.nested(FieldSortBuilder.java:588)
    at org.opensearch.search.sort.FieldSortBuilder.build(FieldSortBuilder.java:411)
    at org.opensearch.search.sort.SortBuilder.buildSort(SortBuilder.java:166)
    at org.opensearch.search.sort.FieldSortBuilder.getMinMaxOrNullInternal(FieldSortBuilder.java:631)
    at org.opensearch.search.sort.FieldSortBuilder.getMinMaxOrNullForSegment(FieldSortBuilder.java:626)
    at org.opensearch.search.internal.ContextIndexSearcher.canMatchSearchAfter(ContextIndexSearcher.java:499)
    at org.opensearch.search.internal.ContextIndexSearcher.canMatch(ContextIndexSearcher.java:491)
    at org.opensearch.search.internal.ContextIndexSearcher.searchLeaf(ContextIndexSearcher.java:309)
    at org.opensearch.search.internal.ContextIndexSearcher.search(ContextIndexSearcher.java:295)
    at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:551)
    at org.opensearch.search.query.QueryPhase.searchWithCollector(QueryPhase.java:341)
    at org.opensearch.search.query.QueryPhase$DefaultQueryPhaseSearcher.searchWithCollector(QueryPhase.java:419)
    at org.opensearch.search.query.QueryPhase$DefaultQueryPhaseSearcher.searchWith(QueryPhase.java:408)
    at org.opensearch.search.query.QueryPhase.executeInternal(QueryPhase.java:263)
    at org.opensearch.search.query.QueryPhase.execute(QueryPhase.java:150)
    at org.opensearch.search.SearchService.loadOrExecuteQueryPhase(SearchService.java:527)
    at org.opensearch.search.SearchService.executeQueryPhase(SearchService.java:591)
    at org.opensearch.search.SearchService$2.lambda$onResponse$0(SearchService.java:560)
    at org.opensearch.action.ActionRunnable.lambda$supply$0(ActionRunnable.java:73)
    at org.opensearch.action.ActionRunnable$2.doRun(ActionRunnable.java:88)
    at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
    at org.opensearch.threadpool.TaskAwareRunnable.doRun(TaskAwareRunnable.java:78)
    at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
    at org.opensearch.common.util.concurrent.TimedRunnable.doRun(TimedRunnable.java:59)
    at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:815)
    at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalArgumentException: features that prevent cachability are disabled on this context
    at org.opensearch.index.query.QueryShardContext.failIfFrozen(QueryShardContext.java:521)
    at org.opensearch.index.query.QueryShardContext.nowInMillis(QueryShardContext.java:555)
    at org.opensearch.index.mapper.DateFieldMapper$DateFieldType.lambda$handleNow$2(DateFieldMapper.java:491)
    at org.opensearch.common.time.JavaDateMathParser.parse(JavaDateMathParser.java:81)
    ... 41 more

Related Issues

Resolves #11248

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Copy link
Contributor

❕ Gradle check result for a615371: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

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

Copy link
Contributor

❌ Gradle check result for 7b8eeec: 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?

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

❌ Gradle check result for 7b8eeec: FAILURE

[org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search/240_date_nanos/date with nested sort now}](https://build.ci.opensearch.org/job/gradle-check/30222/testReport/junit/org.opensearch.backwards/MixedClusterClientYamlTestSuiteIT/test__p0_search_240_date_nanos_date_with_nested_sort_now_/)
[org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search/240_date_nanos/date with nested sort now}](https://build.ci.opensearch.org/job/gradle-check/30222/testReport/junit/org.opensearch.backwards/MixedClusterClientYamlTestSuiteIT/test__p0_search_240_date_nanos_date_with_nested_sort_now__2/)
[org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search/240_date_nanos/date with nested sort now}](https://build.ci.opensearch.org/job/gradle-check/30222/testReport/junit/org.opensearch.backwards/MixedClusterClientYamlTestSuiteIT/test__p0_search_240_date_nanos_date_with_nested_sort_now__3/)
[org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search/240_date_nanos/date with nested sort now}](https://build.ci.opensearch.org/job/gradle-check/30222/testReport/junit/org.opensearch.backwards/MixedClusterClientYamlTestSuiteIT/test__p0_search_240_date_nanos_date_with_nested_sort_now__4/)

Those are indeed flaky till we get it to 2.12 (backport)

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

@gashutos sorry but we have merge conflict here ...

Copy link
Contributor

✅ Gradle check result for 882f1ae: SUCCESS

Signed-off-by: Chaitanya Gohel <[email protected]>
@gashutos
Copy link
Contributor Author

@gashutos sorry but we have merge conflict here ...

Oops, resolved just now.

Copy link
Contributor

❌ Gradle check result for d552d15: 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?

Copy link
Contributor

❌ Gradle check result for d552d15:

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?

Copy link
Contributor

❕ Gradle check result for d552d15: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testStrictWeightedRoutingWithCustomString_FailOpenEnabled

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

@reta reta merged commit 280f6e4 into opensearch-project:main Nov 20, 2023
28 of 29 checks passed
@reta reta added the backport 2.x Backport to 2.x branch label Nov 20, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-11249-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 280f6e488c97e7e4429472b100713d8b7231f38a
# Push it to GitHub
git push --set-upstream origin backport/backport-11249-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-11249-to-2.x.

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

@gashutos could you please manually backport to 2.x? thank you.

gashutos added a commit to gashutos/OpenSearch that referenced this pull request Nov 21, 2023
…oject#11249)

* Handle canMatchSearchAfter for frozen context scenario

Signed-off-by: Chaitanya Gohel <[email protected]>

* Addressig review comments

Signed-off-by: Chaitanya Gohel <[email protected]>

* Enable test for 2.12.0 for nano date now

Signed-off-by: Chaitanya Gohel <[email protected]>

---------

Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Chaitanya Gohel <[email protected]>
fahadshamiinsta pushed a commit to fahadshamiinsta/OpenSearch270 that referenced this pull request Dec 4, 2023
…oject#11249)

* Handle canMatchSearchAfter for frozen context scenario

Signed-off-by: Chaitanya Gohel <[email protected]>

* Addressig review comments

Signed-off-by: Chaitanya Gohel <[email protected]>

* Enable test for 2.12.0 for nano date now

Signed-off-by: Chaitanya Gohel <[email protected]>

---------

Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Chaitanya Gohel <[email protected]>
@lasiltan
Copy link

lasiltan commented Jan 17, 2024

Using the latest docker version (opensearchproject/opensearch:latest) fails with this error. Curiously the latest version of the AWS service does not.

@reta
Copy link
Collaborator

reta commented Jan 17, 2024

Using the latest docker version (opensearchproject/opensearch:latest) fails with this error.

Thank you @lasiltan , that should not be the case, @gashutos could you please double check? Thank you.

@gashutos
Copy link
Contributor Author

The latest image is 2.11.1 version.

[gashutos@dev-dsk-gashutos-1c-2320b8cf ~]$ curl https://localhost:9200 -ku 'admin:admin'
{
  "name" : "e566097573d4",
  "cluster_name" : "docker-cluster",
  "cluster_uuid" : "rKUWMmhWRZmleVAts-28VQ",
  "version" : {
    "distribution" : "opensearch",
    "number" : "2.11.1",
    "build_type" : "tar",
    "build_hash" : "6b1986e964d440be9137eba1413015c31c5a7752",
    "build_date" : "2023-11-29T21:43:10.135035992Z",
    "build_snapshot" : false,
    "lucene_version" : "9.7.0",
    "minimum_wire_compatibility_version" : "7.10.0",
    "minimum_index_compatibility_version" : "7.0.0"
  },
  "tagline" : "The OpenSearch Project: https://opensearch.org/"
}

This fix will be available in 2.12 release -> schedule.

For AWS managed OpenSearch services, we have different release schedule and we patched this fix to 2.11 AOS version itself.

rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…oject#11249)

* Handle canMatchSearchAfter for frozen context scenario

Signed-off-by: Chaitanya Gohel <[email protected]>

* Addressig review comments

Signed-off-by: Chaitanya Gohel <[email protected]>

* Enable test for 2.12.0 for nano date now

Signed-off-by: Chaitanya Gohel <[email protected]>

---------

Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Chaitanya Gohel <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…oject#11249)

* Handle canMatchSearchAfter for frozen context scenario

Signed-off-by: Chaitanya Gohel <[email protected]>

* Addressig review comments

Signed-off-by: Chaitanya Gohel <[email protected]>

* Enable test for 2.12.0 for nano date now

Signed-off-by: Chaitanya Gohel <[email protected]>

---------

Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed bug Something isn't working v2.12.0 Issues and PRs related to version 2.12.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Handle canMatchSearchAfter for frozen context scenario
7 participants