Re: [Tracker] Ready for merge

[Jamie McCracken <jamie mccrack googlemail com>]

On Mon, 2008-08-11 at 15:00 +0100, Martyn Russell wrote:
> Jamie McCracken wrote:
> > On Mon, 2008-08-11 at 14:32 +0100, Martyn Russell wrote:
> > > Jamie McCracken wrote:
> > > > On Mon, 2008-08-11 at 10:55 +0100, Martyn Russell wrote:
> > > > > Have you had time to look at the rest of the branch to see if it is
> > > > > satisfactory?
> > > > unfortunately it is not
> > > >
> > > > Running it I get very poor performance (Im running tracker-applet so
> > > > dont know if that is affecting/pausing it)
> > > Can you elaborate? What is poor performance? The Crawling? The indexing?
> > > The searching?
> > >
> > > > Does not handle file moves (see trunk for this as its non trivial)
> > > Let me come back to you on that one.
> > >
> > > > The gio file monitoring is not adequate - inotify needs optimisation
> > > Why?
> > >
> > > > EG for inotify we only trigger a change if FILE_WRITE_CLOSED is sent -
> > > > that way we know the file has finished changing and we can safely index.
> > > > Currently it indexes whenever the file changes which sucks. 
> > > That's not true.
> > >
> > > We tell the indexer to re-index if we get updates on the file. But if we
> > > get updates more than 5 times, the file becomes black listed and we wait
> > > until we have had 30 seconds of inactivity on the file before emitting
> > > the event up the stack to re-index the file.
> >
> > this facility is for broken apps like bit torrent as well as stuff that
> > frequently changes - its not to be confused with stuff like a file
> > download which would cause 5 reindexes and a blacklist unnecessarily
> > which is unacceptable
> >
> > Beagle uses ïFILE_WRITE_CLOSED along with tracker to prevent this so it
> > is necessary
>
> G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT == FILE_WRITE_CLOSED.
>
> This is a simple fix, we just don't use _CHANGED, but only
> CHANGES_DONE_HINT.
>
> > > Also inotify already has throttling built in so we don't get spammed
> > > with events. There is already a short delay before we get the event.
> > >
> > > We could JUST use G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT event which is
> > > there to suggest it is the last change on a file.
> >
> > If the inotify backend uses ïïFILE_WRITE_CLOSED for that then fine
> > however FAM will never issue that so we need to have optimisations based
> > on whether its inotify or fam (like for the watch count)
>
> So this is fine assuming the fix above.

yes.  for inotify  we use ïG_FILE_MONITOR_EVENT_CHANGES_DONE_HINT, for
FAM we use ïïG_FILE_MONITOR_EVENT_CHANGE

> > > > FAM would
> > > > need a pending list (as per trunk) as it only has the file change and we
> > > > need to wait til changes are stabilised before trigger an index
> > > Again, we have this built in right now. We can improve it of course the
> > > way I suggest above.
> >
> > nope thats misusing it - we want to prevent indexing it until we are
> > sure its stabilised. the blacklist is a worst case safety net. All we
> > are talking about here is not doing any indexing until a certain time
> > period has elapsed
>
> So this point is moot now right (given the fix above)?

well no - FAM is used by solaris and sun ships tracker now so they will
suffer if we dont include some stability checks prior to indexing - the
code for this is simple


jamie