-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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 SearchContext occasionally closed prematurely #5170
Conversation
@@ -169,7 +169,7 @@ | |||
|
|||
private volatile long keepAlive; | |||
|
|||
private volatile long lastAccessTime; | |||
private volatile long lastAccessTime = -1; |
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.
given this I think we should add some checks to the place where this is created in SearchService
ie. this:
SearchContext createAndPutContext(ShardSearchRequest request) throws ElasticsearchException {
SearchContext context = createContext(request);
activeContexts.put(context.id(), context);
context.indexShard().searchService().onNewContext(context);
return context;
}
should look like:
SearchContext createAndPutContext(ShardSearchRequest request) throws ElasticsearchException {
SearchContext context = createContext(request);
boolean success = false;
try {
activeContexts.put(context.id(), context);
context.indexShard().searchService().onNewContext(context);
success = true;
return context;
} finally {
if (!success) {
freeContext(context);
}
}
}
I left one comment but in general this looks great... Can you maybe change the commit message to follow this pattern:
thanks so much! |
Fixes SearchContext from being closed during initialization or immediately after processing is started Closes elastic#5165
@s1monw just updated per your comments regarding the SearchService and the commit message |
looks good I will try merging this in tomorrow! |
pushed thanks so much |
PR for #5165