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 SearchContext occasionally closed prematurely #5170

Closed
wants to merge 1 commit into from

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Feb 18, 2014

PR for #5165

@@ -169,7 +169,7 @@

private volatile long keepAlive;

private volatile long lastAccessTime;
private volatile long lastAccessTime = -1;
Copy link
Contributor

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);
              }
        }
    }

@s1monw
Copy link
Contributor

s1monw commented Feb 18, 2014

I left one comment but in general this looks great... Can you maybe change the commit message to follow this pattern:

Short description (80 chars max)

Long description over multiple lines (80 Chars per line)

Closes #5165

thanks so much!

@s1monw s1monw self-assigned this Feb 18, 2014
Fixes SearchContext from being closed during initialization or immediately
after processing is started

Closes elastic#5165
@jaymode
Copy link
Member Author

jaymode commented Feb 18, 2014

@s1monw just updated per your comments regarding the SearchService and the commit message

@s1monw
Copy link
Contributor

s1monw commented Feb 18, 2014

looks good I will try merging this in tomorrow!

@s1monw
Copy link
Contributor

s1monw commented Feb 19, 2014

pushed thanks so much

@s1monw s1monw closed this Feb 19, 2014
@clintongormley clintongormley changed the title Issue 5165: Fix SearchContext occasionally closed prematurely Fix SearchContext occasionally closed prematurely Jun 7, 2015
@clintongormley clintongormley added the :Search/Search Search-related issues that do not fall into other categories label Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v0.90.12 v1.0.1 v1.1.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants