Re: [Tracker] Using setrlimit() is causing a lot of SIGSEGV

[Jamie McCracken <jamie mccrack googlemail com>]

On Fri, 2009-04-03 at 15:18 +0100, Martyn Russell wrote:
> Jamie McCracken wrote:
> > On Fri, 2009-04-03 at 14:47 +0100, Martyn Russell wrote:
> > > Jamie McCracken wrote:
> > > > On Fri, 2009-04-03 at 12:09 +0100, Martyn Russell wrote:
> > > > > Hi all,
> > > > >
> > > > > I have spent a lot of time this week triaging bug reports in Launchpad 
> > > > > and I noticed ubuntu patches the setrlimit() call we use. I haven't seen 
> > > > > exactly how, but we did some light testing here by not using it at all, 
> > > > > and the files which were causing tracker-extract to crash before, now 
> > > > > are not. The files are, however, being extracted perfectly.
> > > > >
> > > > > I would suggest we remove ALL calls to setrlimit in 
> > > > > trackerd/tracker-indexer/tracker-extract. Certainly the extractor.
> > > > >
> > > > > Does anyone object to this?
> > > >
> > > > yes from experience corrupted files can turn the extract process into a
> > > > zombie
> > > >
> > > > Also memory usage can rocket especially with video files
> > > >
> > > > It might be a case we extend the time permitted for the extractor to
> > > > finish so that slower machines are not penalised but I would object to
> > > > removing this safety net
> > > Time really is not the issue here, but the memory limitation is. Plus we 
> > > already have 2 or 3 mechanics in place to make sure extraction doesn't 
> > > take too long.
> > >
> > > 1. DBus message timeout (10-15 seconds) + kill from indexer
> > > 2. alarm()
> > > 3. setrlimit()
> > >
> > > A lot of bugs and I mean a lot, are being reported due to the memory 
> > > issue not the cpu time issue from what I can see. Carlos' test actually 
> > > backs this up.
> >
> > sure but we should have an upper limit to memory usage?
> >
> > Ive had bugs int he past where tracker-extract caused machines to become
> > unsable because they went into swap
>
> I am not entirely sure setting a memory limit helps. Right now it masks 
> the problem with SIGSEGV and we just get a lot of those. I would much 
> rather someone got their data indexed and then complained about memory 
> usage than having 120 crash reports in a week (as I just have had) 
> mostly down to this limit (as it seems).
>
> Consider this. You mmap a file (to get metadata) as we do in some cases, 
> that file is 150Mb, that instantly causes a SIGSEGV with the current 
> solution.
>
> The limits right now are 128Mb and 512Mb for AMD64 processors (not sure 
> why we have a difference there).
>
> I would like to see this scrapped and to have people report the 
> extractor when it eats too much memory. Then we can take each file on a 
> case by case basis.


I would rather we increase the memory usage permitted rather than risk flames of tracker making their system 
unusable. 

As a compromise, perhaps we could make exceptions for some extractors which we know always uses mmap and are 
therefore less likely to cause swappage?

jamie