Re: [Tracker] Ready for merge

[Martyn Russell <martyn imendio com>]

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.

> > > 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)?

> > > Also we are a differential index so i dont understand why you have
> > > removed the differential update code? What does it do when a file
> > > updates?
> > Not sure what you mean here?
>
> why is the differential function excluded in tracker-db.c in trackerd:
>
> #if 0
>       tracker_db_update_differential_index (old_table, new_table, id, sid);
> #endif
>
> this means that no tags or other metadata added would update the index
>
>
> I also cant find it in tracker-indexer. Did you rename it?

I need to look into this.

-- 
Regards,
Martyn