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

Incorporate fixes and cleanups for Solr 7 #27

Closed
wants to merge 3 commits into from

Conversation

marktriggs
Copy link
Collaborator

Hi there,

I've included three changes here:

  • Some style cleanups--mainly just continuing Tod's good work.

  • The fix to ensure that we don't continue using IndexSearcher objects after they've been handed back to Solr.

  • Simplified lock handling for the HeadingsDB. As I was cleaning up its exception handling, I found it easy to inadvertently introduce deadlocks due to the way I'd originally set up its lock handling. I've replaced that with a much simpler scheme and tested with multiple concurrent users.

The original browse code worked on a model of each BrowseSource having
persistent handles open to the Headings & Auth DBs, and these were
reopened if needed at the point of handling a request.

This doesn't fit terribly well with the Solr API's way of doing
things, where searchers are refcounted and you're expected to return
them when done.  The handler code was mistakenly decrementing the
refcount for the AuthDB searcher but then continuing to use it, which
causes problems when Solr closes the underlying searcher.

This commit changes the request handler to make Browse objects
ephemeral: just get the searcher we need, use them to handle the
request, and return them.  Let Solr handle managing searcher
lifetimes.
@demiankatz
Copy link
Member

Thanks, @marktriggs -- I've created a release-4.1 branch in the repository so that I can get these fixes in place for VuFind 4.1.3 (which still uses Solr 6), and then I have also merged that branch into master (so I can also build a Solr 7-compatible version). Thus, this work is all merged in now!

@demiankatz demiankatz closed this Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants