If there is an intermittent failure to connect (or when using master/slave replication and the master is down) we have a bug since on cron we clear the cached luke data prior to being sure we have refilled it fmor the server. It would be better not to clear it if new data was not fetched.

CommentFileSizeAuthor
#8 protect-cache-435924-8.patch8.29 KBpwolanin
#7 protect-cache-435924-4.patch8.32 KBpwolanin
#3 protect-cache-435924-3.patch7.83 KBpwolanin
#2 protect-cache-435924-2.patch6.87 KBpwolanin
#1 protect-cache-435924-1.patch4.43 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new protect-cache-435924-1.patch4.43 KB

Here's code borrowed from #368245: implement optional caching of query results which should have the desired effect as far as this issue but doesn't cache any query results.

The reason this should work is that the cache is only cleared if an update of some sort succeeds - this will only happen if the master is available. While it's possible there will be a race condition between cron runs, this reduces them.

pwolanin’s picture

pwolanin commented
StatusFileSize
new protect-cache-435924-2.patch6.87 KB

better

pwolanin’s picture

pwolanin commented
StatusFileSize
new protect-cache-435924-3.patch7.83 KB

missed an update in apachesolr_search - refactor to a helper function rather than explicitly using the variable.

JacobSingh’s picture

JacobSingh commented

Looks good, will apply and test

pwolanin’s picture

pwolanin commented

I think we should variable_set() time() or 0 instead of TRUE/FALSE

Then we can add code to not clear set the flag back to 0 cache unless the elapsed time is > the autocommit time (or some reasaonble default?).

Otherwise, a site running cron faster than the autocommit interval will cache stale results.

JacobSingh’s picture

JacobSingh commented

Man, this triggered a huge run-around today unrelated to the actual patch (which seems to work okay).

I think this is good to go.

I'm not too worried about #5, but I suppose it would be an improvement. I think that even trying to clear and re-fill every cron run is not that bad. Perhaps only refreshing when there is an update, while nice, is not needed?

-J

pwolanin’s picture

pwolanin commented
StatusFileSize
new protect-cache-435924-4.patch8.32 KB
pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed
StatusFileSize
new protect-cache-435924-8.patch8.29 KB

minor change to >=

committing to 6.x

pwolanin’s picture

pwolanin commented
Status: Fixed » Closed (fixed)