-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Always include the matching node when resolving point in time #61658
Conversation
Pinging @elastic/es-search (:Search/Search) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we need to disable anything since we keep a reference of the index shard in the local shard reader context ? Are you sure that the problem is the allocation rebalance and not a timeout of the PIT in the test ? If it's not a timeout, we should understand why the reference is not enough when a rebalance occurs. In such case, the PIT should still be valid and the removal of the shard should only happen when the PIT is closed or timed out.
@jimczi I've updated the PR as we discussed. Can you take another look? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one question but the change looks good to me. Thanks for adding the explicit tests for rebalance. That made me think that we should maybe have the same test for deleted indices ?
server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java
Outdated
Show resolved
Hide resolved
@elasticmachine test this please |
@jimczi Sure, will do. If one index of a PIT is deleted, should we allow search to return a partial result when |
By deleted, I meant deleted by the user after the creation of the PIT. In such case the PIT should still be valid since we still have the reference of the index shard in the context ? Regarding partial results, yes I think we should allow to return partial results if some contexts are not found. Isn't it what's implemented today ? |
I think we remove all reader contexts when an index is deleted.
We don't. I will work on it in a follow-up. |
…c#61658) If shards are relocated to new nodes, then searches with a point in time will fail, although a pit keeps search contexts open. This commit solves this problem by reducing info used by SearchShardIterator and always including the matching nodes when resolving a point in time. Closes elastic#61627
If shards are relocated to new nodes, then searches with a point in time will fail, although a pit keeps search contexts open. This commit solves this problem by reducing info used by SearchShardIterator and always including the matching nodes when resolving a point in time. Closes #61627
If shards are relocated to new nodes, then searches with a point in time will fail, although a pit keeps search contexts open. This commit solves this problem by reducing info used by SearchShardIterator and always including the matching nodes when resolving a point in time.
Closes #61627