[Kde-pim] Review Request 110268: Nepomuk Feeder: Simplify change recording logic

Christian Mollekopf chrigi_1 at fastmail.fm
Thu May 2 19:01:11 BST 2013



> On May 2, 2013, 4:35 p.m., Christian Mollekopf wrote:
> > Not sure if we should ever enable changeRecording. The point of those variables was exactly that we can disable the change recoding entirely if the feeder is disabled (set offline). Otherwise the changerecording queue will grow endlessly which will end up as a cpu and memory hog. Change recording could be ok if the feeder is disabled temporariliy (powersaving or whatnot), but it's imo a rather dangerous feature. I'd therefore rather avoid recording changes and rely on the FindUnindexedItemsJob (which could be run whenever the feeder is set online again).
> 
> Vishesh Handa wrote:
>     But the FindUnindexedItemsJob is very expensive and causes HUGE amounts of CPU load. I want to avoid calling it unless absolutely necessary.
>     
>     Right now if we go offline the changerecording will go online and record stuff. Wait, is switching off the agent the same as going offline? Cause then I guess we should just trigger the FindUnindexedItemsJob.

Yes, I switching it off is the same as going offline. Hence the messy workarounds with variables to not set it offline while being able to stop processing (i.e. while on battery) etc.

I fully realize that FindUnindexedItemsJob is very expensive, but at least it's somewhat predictable. If the recording queue grows too large it can cause huge strain on the system as well (not sure if that was fully fixed by david's patches to not rewrite the full queue everytime on disk), and you basically never get rid of that queue as you might be too slow to empty it and it grows endlessy on disk.
AFAIK the changerecording was never meant to be used like this. It is meant as a queue for events which must not be lost (i.e. over a reboot that can happen at any time), giving the client some time to process it. But not as a "let me turn that off and continue processing a week later"-queue. For that it AFAIK doesn't contain the necessary safeguards.


- Christian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110268/#review31897
-----------------------------------------------------------


On May 2, 2013, 8:59 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110268/
> -----------------------------------------------------------
> 
> (Updated May 2, 2013, 8:59 a.m.)
> 
> 
> Review request for KDEPIM and Christian Mollekopf.
> 
> 
> Description
> -------
> 
>    Only record changes when we are not online. We get notified normally
>    otherwise because we are an Observer.
>     
>    Also removed the superflous variables which could be simplified into
>    isOnline()
> 
> 
> Diffs
> -----
> 
>   agents/nepomukfeeder/nepomukfeederagent.cpp 7380a32 
>   agents/nepomukfeeder/nepomukfeederagent.h eafe5cd 
> 
> Diff: http://git.reviewboard.kde.org/r/110268/diff/
> 
> 
> Testing
> -------
> 
> works. Changes are being recorded when we are not online.
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/



More information about the kde-pim mailing list