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

fix explain action on query rewrite #17286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fen-qin
Copy link

@fen-qin fen-qin commented Feb 6, 2025

Description

There is an exception when call explain in "by_doc_id" mode. Response looks like this:

{
    "error": {
        "root_cause": [
            {
                "type": "query_shard_exception",
                "reason": "failed to create query: async actions are left after rewrite",
                "index": "my-nlp-index-1",
                "index_uuid": "I6_pzGG3QBWw0B6KeOJ1pA"
            }
        ],
        "type": "query_shard_exception",
        "reason": "failed to create query: async actions are left after rewrite",
        "index": "my-nlp-index-1",
        "index_uuid": "I6_pzGG3QBWw0B6KeOJ1pA",
        "caused_by": {
            "type": "illegal_state_exception",
            "reason": "async actions are left after rewrite"
        }
    },
    "status": 400
}

The error were caught from TransportExplainAction -> Rewriteable.java because there still be asyncActions left when the flag assertNoAsyncTasks returned as true.

Screenshot 2025-01-29 at 8 29 43 PM

The conflict is down to:

  1. The NeuralQueryBuilder or TermsQueryBuilder doRewrite method introduces asynchronous actions via queryRewriteContext.registerAsyncAction. So it then returns a context with potentially unresolved async tasks.

  2. Rewritable.rewrite then checksassertNoAsyncTasks flag, throws the exception if there exists any async tasks left.

Fix in this PR is resolve this conflict:

  • by moving the query rewrite to coordinator - TransportExplainAction.
  • verify the changes from local using TermsQueryBuilder
    • Before fix
    • Screenshot 2025-01-30 at 9 30 29 PM
    • After fix
    • Screenshot 2025-01-30 at 9 24 10 PM

Related Issues

Resolves #1126

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.

Copy link
Contributor

github-actions bot commented Feb 6, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add instrumentation for inode usage by Opensearch process
1 participant