From kde-pim  Sun Oct 14 01:11:10 2012
From: David Faure <faure () kde ! org>
Date: Sun, 14 Oct 2012 01:11:10 +0000
To: kde-pim
Subject: [Kde-pim] ChangeRecorder's pipeline
Message-Id: <2705049.LYtZxXykSy () asterix ! site>
X-MARC-Message: https://marc.info/?l=kde-pim&m=135017695715029

Hi Volker,

88d868d9a52f1d0 introduced "d->pipeline.enqueue(msg)" in 
ChangeRecorder::replayNext() (which is still there in another form).

But the msg isn't removed from d->pendingNotifications (until the final call to 
changeProcessed), so if saveNotifications is called this message will be saved 
duplicated, won't it?

The handling of these two lists in the Monitor base class seems sane to me:
it's always pipeline = [A B C], pending = [D E] and stuff moves towards the 
left: A gets emitted when data is available, D moves in after C in 
dispatchNotification, and saving at that point would lead to [B C D E], all fine 
there.

But in ChangeRecorder the items get duplicated into the pipeline, and removed
from the pipeline when data is available, and later on removed from 
pendingNotifications when changeProcessed() is called. It took me 2 hours to 
find this :-}    (more details below)

It's almost fine to "abuse" the two lists that way, but I think 
saveNotifications() should then skip the pipeline when called from a 
ChangeRecorder, to avoid saving duplicates.
However, in addition, this makes things harder for any shared code that uses 
pipeline and pendingNotifications --- like what I plan to do, to speed up 
saving.
But I'm not sure if this can be done differently...
Maybe with a third list? Let's call it "emitted" and put it on the left:

emitted = [A], pipeline = [], pending = [B C D E]
where "emitted" is the one waiting for changeProcessed().
After that we get
emitted = [], pipeline = [B], pending = [C D E]
while waiting for dataAvailable in the Monitor base class, and before moving B 
to emitted.

In a pure Monitor emitted would always be empty, in ChangeRecorder it would 
have at most one message. Saving would save all three lists.

I think this would be easier to understand and manage than the current 
solution:
 pipeline = [], pending = [A B C D E]
which replayNext() turns into
 pipeline = [A], pending = [A B C D E]
and which MonitorPrivate::flushPipeline() turns back into
 pipeline = [], pending = [A B C D E]
again before emitNotification() and while waiting for changeProcessed(), which 
finally removes A from pending. Tricky, isn't it? :-)

My first try was to just remove A from the pending list in replayNext(), but 
this would lose data if the process crashes after the data is available 
(emitNotification) and before changeProcessed().

Please correct me if any of this analysis is wrong, and please voice any 
better suggestion you might have for both
1) not saving duplicates
2) using these lists in a consistent way between both classes, in order to be 
able to add more logic in the saving code.

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

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