Akonadi: what should trigger fetching the full payload on new items resource-side?

[Daniel Vrátil]

On Friday, 24 September 2021 19:57:43 CEST Friedrich W. H. Kossebau wrote:
> Am Freitag, 24. September 2021, 17:25:47 CEST schrieb Daniel Vrátil:
> > On Friday, 24 September 2021 14:14:49 CEST Friedrich W. H. Kossebau wrote:
> > > (cc: Daniel, having happily seen he might again have some spare
> > > resources
> > > for Akonadi? :) )
> > 
> > Let's hope it lasts :D
> 
> I do hope :)
> 
> ((On that matter, next to the question of this email, I have another open
> task where your assistance would be good to have, even more as you started
> that task:
> completing the introduction of the batch item retrieval, and getting rid of
> the current
>     ResourceBase::retrieveItem(const Akonadi::Item &item, ..,)
> API ideally in time for 21.12
> I already adopted a few simple local filesystem-based resources to it, with
> one MR currently still open (kdepim-runtime MR 31). But the remote ones
> (IMAP, DAV, openXChange, tomboy) will need more clever people working on
> it, so perhaps you can find some interest in completing this open task as
> well? Happy to rather assist you here.))

I suppose I could take a look at that, yeah :-) I should probably somehow 
prioritize the implementation of People API in KGAPI, now that Glen has 
brought that up, unless someone steps up to do that. Not the kind of task I 
was looking to use up my newly gained motivation on...

> > > I found the following currently broken in Akonadi:
> > > 
> > > given a client monitoring items with a fetchscope with fullPayload, when
> > > a
> > > resource is bringing in a new collection with items in it and calling
> > > ResourceBase::synchronize() to make that known, currently nothing
> > > triggers
> > > the fetching of the payload into the server and thus the clients, so ETM
> > > e.g. filters them out.
> > > 
> > > Example:
> > > Add "Personal Contacts" with existing vcf files in
> > > ~/.local/share/contacts/. The contacts will not be displayed at current
> > > runtime, only after a restart of Akonadi and its clients, which seems to
> > > trigger some re-synchronization (not yet researched).
> > 
> > Although the case you are describing is valid, this example should work,
> > since the vcard resource always sets a full item payload during item sync
> > (see VCardResource::doRetrieveItem()/VCardDirResource::doRetrieveItem()),
> > so if the Items are not showing up in clients, the problem is probably
> > elsewhere. You might be tracking two bugs here.
> 
> The "Personal Contacts" resource is that contactsresource though, which is
> something else than the vcarddirresource :) (also first had to sort that
> out).

Aaah, good point, should've read that more carefuly, sorry. Indeed the 
"Contacts resource" does push empty items into Akonadi and leaves it up to 
clients to fetch the payload on demand. I suppose that's an optimization to 
avoid unnecessarily duplication of local and always-available data in Akonadi.

<sidenote>
I do wonder if we need three resource implementations to support local 
contacts. Wouldn't a single Contacts Resource that supports multiple formats 
make more sense from user's point of view?
</sidenote>

> 
> The vcarddirresource is missing out some additional things on creation of a
> resource, which I though already have a plan for to handle, once this issue
> here is solved. So I stick to the contactsresource as example here.
> 
> And ContactsResource::retrieveItems(const Akonadi::Item::List &items, const
> QSet<QByteArray> &parts) is never called during the creation of a resource,
> as a result of nothing server-side seemingly being interested to satisfy
> the fullPayload flag set by the monitor. Things just end after
> ContactsResource::retrieveItems(const Akonadi::Collection &collection) has
> delivered all the new (payloadless) items to the server.

retrieveItems(const Item &item, const QSet<> &parts) is very special: it only 
gets invoked by the Akonadi Server via DBus when the server realizes that it's 
missing (part of) the payload. It cannot get triggered during resource 
creation or during sync.

> 
> > > Now I wonder what logic in the current design should ensure and how that
> > > the payload is actually also pulled when a new item is created in the
> > > server and there are monitors subscribed which want the full payload.
> > 
> > If the Item doesn't have a full payload in Akonadi, it should be retrieved
> > on demand whenever a client requests an operation that requires the full
> > payload (usually an ItemFetchJob with fullPayload fetchscope, or
> > ItemCopyJob and ItemMoveJob, in  some cases).
> > 
> > Historically, Akonadi::Monitor would internally ensure this, if the
> > fetchScope of the Monitor was to fetch full payload and it received a
> > notification about a new Item, the Monitor would schedule an ItemFetchJob
> > with the same fetch scope and the server, upon finding that the client has
> > requested more than what's stored in Akonadi, would fetch the payload form
> > the resource and provide it to the Monitor. This way the Monitor was able
> > to emit itemAdded() signal with full payload, even if the initial sync
> > only
> > synced items without payload.
> > 
> > The new Akonadi Server-side notification payload feature may have broken
> > this, since the Monitor no longer uses ItemFetchJob - it should always
> > received the entire Item already as part of the notification. The
> > NotificationManager should take care of retrieving the missing payload, if
> > there's at least one client who wants it. I would probably dig in here
> > first, to see whether that's really the case.
> 
> one thing which caught my attention was that
> ItemCreateHandler::sendResponse() in some cases also would use
> ItemFetchHelper to ask during the protocol for the full payload. 

The ItemCreateHandler always returns the contents of the newly created/merged 
Item to the client. While in the "create new Item" we have everything we need 
in the handler (since the client must have had provided us with the entire 
Item) and we could just send that back to the client, the "merge" mode is more 
tricky: in this case the client may only send some parts of the Item, and we 
would need to figure out what's the final state after the merge to send back the 
whole thing. The easiest way is to use the ItemFetchHelper here to do that...

> Though not
> in the case when a resource is running the ItemCreateJob as part of the
> retrieveItems(Collection) call.
> When I first saw this, my initial thought was, okay, so the
> ItemCreateHandler should check the global notification fetch scope here and
> then ask the resource for all the missing payload data during the
> ItemCreate protocol. Would that make sense, or is there chance of a
> deadlock? 

The rule of thumb is, when the request is coming from a Resource, the Server 
must not interact with the Resource in any other way, since that could indeed 
deadlock  or lead to some endless loops

<sidenode>
My goal for the next months is to finally, after several years, complete the 
new "job" API for Akonadi libraries, which will then be a foundation for a new 
ResourceBaseV2, which will allow more advanced scheduling and even executing 
more things in parallel. This should make it possible for the server to 
interact with the resource even during sync.
</sidenode>

In the case we are debugging here, the Resource doesn't care about the result 
of the command, though. It only wants to know whether the ItemCreateJob was 
successful or not. It's the regular clients who should receive an itemAdded 
notification, containing the Item with the requested payload. This assumes 
there's at least one client with a Monitor that has fullPayload fetch scope 
set, which almost certianly there is.

> Still missing some parts of the bog picture, so unsure whether
> doing full payload query calls at this state would run into some issues at
> least with some resources, e.g. because there is that one-task-at-a-time
> thing with agents/resources I have yet to understand.

Exactly, that's the blocker here. We can't execute another task 
(retrieveItem()) until the current one (itemsync) has finished in the resource.

Before notifications payloads, the Monitor would have received the itemAdded 
notification, would  have tried to fetch the Item from Akonadi Server, which 
would then retrieve the payload from the resource, once the resource have 
finished syncing. This lead to some weird behavior, like seeming nothing 
happening for a while (as all the ItemFetchJobs in Monitor were stuck waiting 
for Akonadi Server, which in turn was waiting for the resource to finish sync 
and then delived payloads for the items, one-by-one), so then they would 
slowly start appearing really only after the sync was over. The batch 
retrieveItems() may have helped to mitigate it a little as it allowed the 
resource to return the payload in batches, but still.

With notifications payloads, the server promises to deliver the entire Item to 
the Monitor. There is code in NotificationCollector that should ensure that 
Akonadi has all requested parts of the Item available, and use ItemFetchHelper 
to retrieve missing parts if needed. However the case when the 
NotificationCollector is invoked in a Resource context (i.e. triggered by the 
resource creating a new Item) may prevent the ItemFetchHelper from requesting 
the payload from the resource, leading to the bug you mention here.

There are some ways to fix this, but I wonder whether there's any point in the 
Contacts Resource doing this optimization where it doesn't send the payload, 
since someone will almost definitely want the full payload, and will ask the 
resource to deliver it for every single item almost immediately after 
sync...maybe instead it could just upload the payload like any other resource 
does, and just set a short cache expiration policy, so that the Akonadi's 
CacheCleaner will expire the parts soon after, avoiding the long-term 
duplication of storing the contact once in a VCF file and once in the 
database...the difference here is, that expired part remains in the databse, 
just its content is truncated to 0 bytes, which is different from the Contacts 
Resource not uploading any payload part at all. 


/Dan

> 
> Cheers
> Friedrich


-- 
Daniel Vrátil
www.dvratil.cz | me at dvratil.cz
facebook.com/danvratil | @danvratil | +DanVrátil

GPG Key: 0x3A850307F5065B61
Fingerprint: 76C92F085D0D6F9E5AD42BFD3A850307F5065B61
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20210924/d77dea30/attachment.sig>