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

Introduce search context - point in time view of indices #56480

Closed
wants to merge 98 commits into from

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented May 9, 2020

Open a new search context:

POST /twitter/_search_context?keep_alive=5m

Execute a search with a specific search context

POST /_search
{
    "size": 100,
    "query": {
        "match" : {
            "title" : "elasticsearch"
        }
    },
    "search_context": {
       "id":  "46ToAwEHdHdpdHRlchZSa1dRZDFELVNLbTZkQVJudUtGMFFnAAAWNVAwS09JWTdTRUdibWE1ZC1id0tjQRRURnhPWjNFQmI3bmVOeVZReS1tRAAAAAAAAAAB",
       "keep_alive": "5m" 
    }
}

Close a search context when it's no longer needed

DELETE /_search_context
{
    "id" : "46ToAwEHdHdpdHRlchZSa1dRZDFELVNLbTZkQVJudUtGMFFnAAAWNVAwS09JWTdTRUdibWE1ZC1id0tjQRRURnhPWjNFQmI3bmVOeVZReS1tRAAAAAAAAAAB"
}

Notable works in this change:

Relates #46523
Relates #26472

Co-authored-by: Jim Ferenczi jimczi@apache.org

dnhatn and others added 30 commits January 27, 2020 15:38
With this change, we partially move the state of SearchContext to 
ReaderContext. This is another step allowing us to move the state of
search to the coordinating node.

We will need several follow-ups to move the entire search state to the 
coordinating node.

Relates #46523
With this change, we partially move the state of SearchContext to
ReaderContext. This is another step allowing us to move the state of
search to the coordinating node.

We will need several follow-ups to move the entire search state to the
coordinating node.

Relates #46523
This commit moves the states of search to the coordinating node instead 
of keeping them in the data node. 

Relates #46523
@dnhatn
Copy link
Member Author

dnhatn commented Jul 3, 2020

@mayya-sharipova I think we have addressed your comments. Would you please take another look?

@mayya-sharipova
Copy link
Contributor

@dnhatn @jimczi Thanks for addressing my comments.

I was trying to test recent changes with alias filters, and I am getting the following error:

"can't read named writeable from StreamInput"

java.lang.UnsupportedOperationException: can't read named writeable from StreamInput
        at org.elasticsearch.common.io.stream.StreamInput.readNamedWriteable(StreamInput.java:1121) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
        at org.elasticsearch.common.io.stream.StreamInput.readOptionalNamedWriteable(StreamInput.java:1146) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
        at org.elasticsearch.search.internal.AliasFilter.<init>(AliasFilter.java:51) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
        at org.elasticsearch.common.io.stream.StreamInput.readMap(StreamInput.java:641) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
        at org.elasticsearch.action.search.SearchContextId.decode(SearchContextId.java:113) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
        at org.elasticsearch.action.search.TransportSearchAction.lambda$executeRequest$2(TransportSearchAction.java:221) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]

I think that error happens because ByteBufferStreamInput that is used in SearchContextId::decode is not NamedWriteableAwareStreamInput and can't read AliasFilter::new.

@dnhatn
Copy link
Member Author

dnhatn commented Jul 4, 2020

"can't read named writeable from StreamInput"

Thanks Mayya. 4a38f0d should address the issue.

@mayya-sharipova
Copy link
Contributor

@dnhatn Thanks for the latest changes, open context search on alias filter and multiple indexes works well for me! I have also checked the latest code changes, and everything LGTM!

@dnhatn dnhatn requested a review from jpountz July 9, 2020 15:05
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

This PR is very large so I focused on the high level and it makes sense to me. I wonder what is the motivation for marking it beta, this looks straightforward enough to me to not feel the need to mark it beta?

docs/reference/search/search_context.asciidoc Outdated Show resolved Hide resolved
docs/reference/search/search_context.asciidoc Outdated Show resolved Hide resolved
@jpountz
Copy link
Contributor

jpountz commented Jul 16, 2020

Another thing is that the docs make it obvious that it works with _search but they don't make it obvious that it works with _async_search too.

@dnhatn
Copy link
Member Author

dnhatn commented Jul 28, 2020

Jim and I discussed this PR offline. We decided to take a different approach. Thanks, everyone!

@dnhatn dnhatn closed this Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants