Olly, thanks for your feedback. On Mon, Jul 31, 2017, at 23:29, Olly Betts wrote:> On Mon, Jul 31, 2017 at 09:24:29AM +0200, Robert Stepanek wrote: > > We'd appreciate any hints on how to fix this. I've written up our > > findings and solution attempts below. Should we post this on trac? > > Yes, it'd be good to have a ticket to track this.I've created ticket #752 (https://trac.xapian.org/ticket/752).> > Should we probably keep it and make QueryOptimiser take ownership? > > That's exactly how I fixed a similar earlier issue (see commit > 2299e1d21e39f1295c81833ccd5037f746f4744a). > > We should probably address both issues consistently. If setting the hint > to NULL works, we should probably evaluate that approach as it avoids > the overhead of tracking ownership and of checking if we're about to > delete the current hint.We are testing the patch to set hint to NULL currently. I'll wait this week to see if we experience any crashers related to it. I am not deep enough into the query optimiser code to know which approach is better in term of performance. If taking ownership is preferred I could make the context resetter aware which postlist member to skip during the free(). Cheers, Robert
On Wed, Aug 02, 2017 at 03:06:27PM +0200, Robert Stepanek wrote:> I've created ticket #752 (https://trac.xapian.org/ticket/752).Thanks. I'll continue the technical discussion there.> We are testing the patch to set hint to NULL currently. I'll wait this > week to see if we experience any crashers related to it.It's been a week - how does that patch look? It would be good to get a fix for this out, and it's really about time we released 1.4.5 anyway. Setting the hint to NULL seems like it should be an improvement over the current situation, so we could certainly do that for 1.4.5 if we don't have a conclusion on which approach to take in the longer term. Cheers, Olly
On Tue, Aug 8, 2017, at 07:13, Olly Betts wrote:> On Wed, Aug 02, 2017 at 03:06:27PM +0200, Robert Stepanek wrote: > > We are testing the patch to set hint to NULL currently. I'll wait this > > week to see if we experience any crashers related to it. > > It's been a week - how does that patch look?We are running it in production since Friday and haven't had any crasher, so it's looking good! I had planned to wait until Thursday/Friday to report back. Overall, I am highly confident that the patch fixes it, we had at least one crash a day before.> Setting the hint to NULL seems like it should be an > improvement over the current situation, so we could certainly do that for > 1.4.5 if we don't have a conclusion on which approach to take in the > longer term.Yes, I think setting to NULL it the simplest approach to take in the short term. And we haven't seen any performance degradation on our search layer since. Cheers, Robert