Bug 72112 - PBAP: suspend+resume
Summary: PBAP: suspend+resume
Status: RESOLVED FIXED
Alias: None
Product: SyncEvolution
Classification: Unclassified
Component: PBAP (show other bugs)
Version: 1.4
Hardware: Other All
: medium enhancement
Assignee: Patrick Ohly
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 70950
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-28 10:08 UTC by Patrick Ohly
Modified: 2014-04-14 10:44 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Ohly 2013-11-28 10:08:56 UTC
Currently SyncEvolution uses a combination of GetSize() and PullAll() without parameters. There have been suggestions to break up the download into chunks with Offset and MaxCount parameters to PullAll().

It is unclear what that is supposed to achieve, so use cases are needed which can only be implemented that way.

It is worth pointing out in this context that uint16 is used in the PBAP API, including the GetSize() used by SyncEvolution. It is unclear if or how more than 64K contacts on a phone can be synced.

At the moment, SyncEvolution probably stops at the limit given by GetSize() even if PullAll() returns more contacts (untested!).
Comment 1 Patrick Ohly 2014-01-14 13:23:37 UTC
The purpose of downloading in chunks is that this allows suspending and resuming the sync. The actual goal of this issue, regardless how it is implemented, is to add a Suspend/Resume() method to PIM Manager syncing.
Comment 2 Patrick Ohly 2014-01-14 13:39:34 UTC
Solution 1: chunking
====================

One way to achieve suspend/resume would be to split the transfer into chunks. SyncEvolution then can pause after each chunk. I am not a fan of this, for several reasons:
Make the chunks to large and suspending will only occur after a noticable delay (to be tested).
     1. Make them too small and there will be additional overhead (again, to be tested).
     2. The definition of contact offsets is poor, so when contacts get deleted in parallel to a transfer, the outcome is unclear. Phones might crash and/or return incorrect data, leading to duplicates or missing data after the sync.
     3. 
Testing the last scenario with all supported phones would be a huge effort, so I'd prefer to avoid it.

Solution 2: streaming
=====================

I implemented support for using a named pipe instead of a plain file. In this case, obexd writes into the pipe and SyncEvolution reads from it. To suspend, SyncEvolution simply stops reading.

The expectation is that this also stops the actual transfer via Bluetooth because obexd gets stalled writing the data fairly quickly (buffers in a pipe are small). This does work, albeit not as gracefully as one would hope. The transfer does indeed stop and can resume seamlessly later, but obexd freezes completely in a blocking write() call during that time. In other words, *all* operations of obexd get "suspended"; it also becomes unresponsive on the D-Bus session bus.

Streaming has one additional advantage over a plain file: there is no need to buffer the entire address book in a file. SyncEvolution can (and does) consume contacts as they come in with plain files and the pipe, but with the file-based approach it cannot discard the already processed data.

Solution 3: dedicated Bluez API
===============================

The Transfer instance has a Cancel() method. Perhaps Suspend() and Resume() can be added there, too?

-------------------------------------------------------

My personal preference would be option 2, with writing in obexd changed such that it can detect full pipes and continue servicing other operations until the pipe becomes usable again. I prefer that because streaming is more efficient and thus worthwhile on its own. The next step would be to support fd passing and thus allow avoiding named pipes in the file system, which are a security risk (all apps of the user have access) and potential resource leak (in case of failures).

But ultimately the Bluez maintainers need to weigh in and at least provide guidance about the pefered approach.
Comment 3 Patrick Ohly 2014-01-14 14:57:49 UTC
The problem with downloading in chunks is that the indexing is not clearly defined. Suppose that a session starts and enumerates contacts from #0 to #99.
Now suppose that contact #49 gets deleted. When asking for 50 contacts starting at #0, what is the response?

It could be either
a) 49 contacts (#0 to #48)
b) 50 contacts (#0 to #48 and #50)

Now, where should the next chunk start? In case a), it probably has to be at offset #50. If we do that in case b, will we get contact #50 again (duplicate after sync)?

A second sync would correct that mistake, though.
Comment 4 Patrick Ohly 2014-01-15 13:52:24 UTC
Luiz pointed out that an explicit suspend API in Bluez may take advantage of OBEX enhancements and thus help avoid timeouts in peers when not telling them that we suspend. The exact API still needs to be determined.

Either way, a new PIM Manager API is needed for suspend/resume. See the proposal for SuspendSync()/ResumeSync()/"status = suspended" in bug #70950 comment #1. Is that okay?
Comment 5 Patrick Ohly 2014-02-03 13:47:48 UTC
(In reply to comment #4)
> Either way, a new PIM Manager API is needed for suspend/resume. See the
> proposal for SuspendSync()/ResumeSync()/"status = suspended" in bug #70950
> comment #1. Is that okay?

I went ahead and implemented the API by suspending the local database side of the sync. The Bluetooth transfer continues, we still need a new API to also suspend that.

Note that I added a "bool" return code to Suspend/ResumeSync():

         Returns true if and only the suspend state changed, otherwise
         false.
Comment 6 Patrick Ohly 2014-04-14 10:44:56 UTC
SyncEvolution 1.4.99.1 also uses Bluez >= 5.15 Suspend/Resume to really freeze the transfer.

  By default, the new API freezes a sync by stopping to consume data on the
  local side of the sync.

  In addition, the information that the sync is freezing is now also handed
  down to the transport and all sources. In the case of PBAP caching, the local
  transport notifies the child where the PBAP source then uses Bluez
  5.15 Transfer1.Suspend/Resume to freeze/thaw the actual OBEX transfer.

  If that fails (for example, not implemented because Bluez is too old
  or the transfer is still queueing), then the transfer gets cancelled
  and the entire sync fails. This is desirable for PBAP caching and
  Bluetooth because a failed sync can easily be recovered from (just
  start it again) and the overall goal is to free up Bluetooth bandwidth
  quickly.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.