#22 new
Tom Coleman

Add an :ignore_errors config flag

Reported by Tom Coleman | October 6th, 2009 @ 08:08 PM | in Wishlist

Comments and changes to this ticket

  • Tom Coleman

    Tom Coleman October 6th, 2009 @ 08:10 PM

    Adds a new
    configuration flag, which means that any rsolr errors that result from
    Sunspot::Rails actions which are implicitly triggered (such as saving
    or deleting a document) will be logged but not propagated.

    This means that if the solr server goes down, the rails server will
    continue to operate as normal (obviously if you try to run searches
    you will have a problem---the errors will propagate there), which if
    as in my case, searching is a low-priority aspect of your site, is a
    good thing.

    Again, I don't have a test here, although this one would be even more important to test properly..

  • mat

    mat October 8th, 2009 @ 02:55 PM

    Hey Tom,

    This is good work, but I'd rather include this functionality in Sunspot itself - and flesh it out so that, for instance, calling #search would return an empty search (no results, facets, etc). Then Sunspot::Rails would just read the appropriate config from sunspot.yml and pass it on to Sunspot itself.

    As for testing, it'd be pretty impossible to do integration specs, but using RSpec's mocks to raise the appropriate exceptions in the api specs would probably be sufficient.

    Let me know if you'd like to work on that - otherwise I'll try to tackle it myself at some point.

    Mat

  • Tom Coleman

    Tom Coleman October 8th, 2009 @ 07:55 PM

    Yes, this is a good point, the code does really belong in sunspot rather than sunspot_rails. My initial implementation is a little hacky I'd admit.

    I'm happy to work on it next week sometime, but I'd like to get a better idea of exactly how you think it should work.

    In the ideal world, what I'd like to happen is that if the solr server is down, the following two things happen (from a rails perspective)

    1. On index updating actions (i.e. an add or delete) no exception is thrown, but an error is posted in the rails logs.

      • Not sure exactly what should happen here. Perhaps session.index and related can return false or something if there is an timeout (and the config is set) and sunspot_rails can log a message if it sees that.
    2. On a search action, an exception is thrown. On the other hand it doesn't really matter if, as you suggest, some kind of 'special' search result is returned. I can just look for that and throw an exception myself. But I probably would push for an exception here---you wouldn't want people to be confused into thinking there were no search results, would you?

    Let me know what you think.
    Tom

  • benjamin

    benjamin October 14th, 2009 @ 01:17 PM

    +1 to add it to sunspot, instead of sunspot_rails .. however, after some considerations, i would not return an empty result
    array, if solr is not available, but rather raise an appropriate
    exception like Sunspot:SolrUnavailable.

    I think the app should deal with that situation, as it exceptional.
    returning an empty array would send people on a wrong trail, when
    debugging.

    we should add a raise_connection_errors config flag to sunspot.yml.
    ignore_errors is too vague, as i want to see certain errors like
    solr-query syntax errors..

    furthermore we should know what to do with out-of-sync records in
    the rails app. probably queueing all records, that were not reindexed
    because of Sunspot:SolrUnavailable errors.

    Cheers
    Ben

  • Tom Coleman

    Tom Coleman October 15th, 2009 @ 08:13 PM

    Basically the equivalent of index_orphans ? Maybe database_orphans..

    And the method would be add_database_orphans I suppose.

    In my case, this would happen rarely enough that I wouldn't really care, I'd just reindex the whole db. Then again, for a large enough deployment this could be a serious performance problem...

    I try to have a look at this early next week..

  • Tom Coleman

    Tom Coleman October 18th, 2009 @ 10:58 PM

    Mat or Benjamin,

    Can I get some clarification on this? What are we looking for sunspot to do if this flag is set, and you try to index a class?

    Return the exception when it would normally return nil? (we need to be able log the exception in sunspot_rails).

    Feels a bit strange...

  • mat

    mat October 19th, 2009 @ 09:52 AM

    Having given this more thought, I have a clearer idea of how I'd like to do this. In particular, I think Sunspot::Rails should do the error handling, leaving pure-Sunspot users (if any exist!) to handle errors using good ol' Ruby. Here's what I propose:

    1) Add a Sunspot.session= method, which allows clients to manually set Sunspot's singleton session. This will allow clients (e.g. Sunspot::Rails) to do custom session handling, while maintaining consistency between higher-level client calls (e.g., Post.search) and direct calls to the Sunspot singleton (e.g., Sunspot.search(Post)).
    2) Add a Sunspot::Rails::SessionProxy class, which implements the public API of Session, but provides several higher-level session management features. Notably: one-session-per-thread, separate master/slave sessions (if configured), and graceful error handling (if configured). Thus if the SessionProxy gets a Solr exception back, it can log errors to the Rails log (since it's Rails-aware) and recover gracefully, if so configured. This SessionProxy would then be set as Sunspot's singleton session in the Sunspot::Rails plugin's initializer.
    3) Modify the Sunspot::Search class so that it is in a consistent state even if a search has not been successfully executed (all the public methods return appropriate empty responses). That way, if calling Search#execute! raises an error, and that error is caught, the client can continue to work with the Search object.

    Thoughts all?

  • Tom Coleman

    Tom Coleman October 19th, 2009 @ 07:21 PM

    Sounds great, although I am still not sure about 3)? Who is catching the error? Sunspot::Rails? or the rails app itself?

    In the second case, I don't see how we can get the search object back out again... code will look like

    begin

    search = Post.search { .. }
    

    rescue Sunspot::SolrUnavailable

    # pass
    

    end

    which will of course run but I don't think that search will ever get assigned to a value. (Correct me if I am wrong).

    In the first case, I would think that this could be VERY confusing to users who don't know what's going on "I'm getting a search object back, but with no results... that's weird, I thought there was data in the index, I must be getting the search syntax wrong somehow....".

  • benjamin

    benjamin October 21st, 2009 @ 07:33 AM

    i agree with tom that exception handling should be part of the application, not sunspot or sunspot rails. We should raise a certain exception and probably have a hook for empty results sets.. e.g.

    begin

    @results = Post.search {}
    

    rescue Sunspot::Rails::SolrUnavailable

    flash[:error] = "We're sorry, the search server is down"
    @results = Sunspot::Util.empty_result_set
    

    end

    so the app can decide how to handle this situation. So i guess the Sunspot::Rails::SessionProxy would be right way to go, and it should pass the exceptions to the application. That behavior can be handled by a configuration flag, like rais_error = true|false, if you don't care about exceptions.

    This should work for the after_save hooks as well as for searching.

    Cheers
    Ben

  • benjamin

    benjamin October 21st, 2009 @ 05:22 PM

    • Tag set to rails
  • Tom Coleman

    Tom Coleman November 14th, 2009 @ 07:59 AM

    Guys,

    I'm not really sure what the state of this bug is. Browsing the source code, it seems that points 1 + 2 that Mat raised have been implemented, which is great.

    However, coming back to this and reading over the conversation again, I think we drifted away from my original intention with the patch. Although it would be great if sunspot threw some more sensible errors (rather than a generic RSolr::RequestError), and provided help for a site that was dealing with such problems, this is not really the issue that I was trying to solve.

    The issue that I was dealing with was due to the automatic indexing of models that is triggered by (:auto_index/:auto_remove). Although this is very useful, it is a problem if it throws errors when models are created if Solr is down. If (as in my case) making sure things save is very important, but making sure things are indexed isn't, this patch is quite useful.

    Thoughts?


    I've attached a patch which implements the database_orphans method that we discussed before.

  • Tom Coleman

    Tom Coleman November 14th, 2009 @ 08:00 AM

    Also I think that there is an error in the logic of the index_orphans method (setting count to self.count). What if there are no records in the database?

    We should use the solr count (I'm no sure what the easiest way to get that is?)

  • mat

    mat December 27th, 2010 @ 02:48 PM

    • Milestone changed from Feature Requests to Wishlist
    • Tag changed from rails to feature, patch
    • Milestone order changed from “0” to “0”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Awesome Solr interaction for Ruby

People watching this ticket

Tags

Pages