[Nepomuk] Bugs, Patches and Scripts. - StrigiService & KInotify

[Sebastian Trüg]

Hi Vishesh,

On 04/26/2010 01:33 PM, Vishesh Handa wrote:
> I was going through the Strigi Service, and I noticed that it stores the
> folders to index in a service file named strigiservicerc (duh!) The
> problem is that the folder being watched are sometimes renamed/moved,
> and when that happens Strigi deletes its indexed metadata. Furthermore,
> if a directory in one of the indexed directories, and is moved out of
> it, the metadata is deleted. :-/ This all happens when the strigiservice
> is restarted. The culprit ->
> Nepomuk::IndexScheduler::removeOldAndUnwantedEntries()
> 
> Fixing this, in my opinion, would require us to watch the indexed
> directories for changes. Unfortunately crappy /inotify/ doesn't allow
> this. If a watched directory is renamed it gives us a EventMoveSelf, but
> doesn't tell us what it has been renamed to or where it has been moved.
> (I know! Stupid!) So, the only solution I can think of is to watch its
> parent directory, that will atleast tell us what it's been renamed to.
> In order to know where it has been moved to we'll need to watch
> everything, which is a huge resource hog (Yes, Filewatcher, I'm talking
> about you!)

oh, yes, the filwatcher and its mem leak....

You are right about the config update. I did not think of that. Nice catch.

> For starters, we need to move the kinotify class to a better location,
> so that it can be used by both the filewatcher, and strigi.

While conceptually you are completely right I think it would be better
to make the FileWatcher update the strigi config. FileWatcher is taking
enough resources as it is by watching all the folders.

> In KINotify, I found a couple of functions to be implemented
> inefficiently ( O(n), when it can be close to O(1). ) So, I optimized it
> a little bit.

I thought about doing it this way, too. But then twice the mem is used
and I figured watchingPath() and removePath() are not called very often.
Thus, the mem savings would be preferable. I suppose...

> The problem came while optimizing /KInotify::removeWatch(
> const QString& path )/. It currently removes all watches who's paths
> start with path, which doesn't seem right. It should /only/ remove the
> watch corresponding to the given path. The full optimization patch does
> this.

Why do you think it should do this? After all, addWatch does also
recurse into subdirs. Plus, we only stop watching a folder if it is
removed. In that case the subfolders are of no interest anyway. :P

> I've tried to hack a solution to the *BUG*. This is just a preliminary
> patch! I created a new class called ConfigFolderWatcher (got a better
> name?). I didn't want to clutter up the /strigiservice/ class. In order
> for this class to work I copied the kinotify class from the filewatch
> service (Temp. solution, you'll need to do the same) The current Patch
> works perfectly for renaming and moving to a sibling directory. But not
> for, well, anything else. Look at the source code! The filewatcher
> service suffers from a somewhat same problem.

Only sibling? AFAIK the only problem is moving to a non-watched folder,
i.e. a folder outside the home dir or any indexed folder.

About the ConfigFolderWatcher class:
As mentioned above I think it should just be put in the file watcher.
The strigi service does watch the config file anyway and update
automatically.
Thus, it can be simplified a lot by removing all the dirwatch details
and just keeping the slotMoved method. So in the end it is just one
method. Maybe no need for a class. :P

> I usually tend to write scripts for stuff I do frequently, and I wrote a
> couple for Nepomuk. They are available over here ->
> http://pastebin.com/pxDVSWWQ . The service ones aren't perfect but
> they're useful. Has anyone else written any scripts which may be useful?
> If so, please share them! :-)

I am currently working on a full blown test suite for the nepomuk
services. The first one is the strigi service and the basic env is done.
It starts a new dbus/kde session and runs the test in there before
erasing the temp kde session. So far these scripts are in the Mandriva
svn but I will see about making that public so we can collaborate. :)

Cheers,
Sebastian