On Mon, 2012-02-06 at 21:29 +0100, Patrick Ohly wrote: > I'm currently experimenting with a different approach for handling the > 409 in the binfile client: when an Add fails with 409, catch it as it is > done at the moment, but then tell the server that it was mapped to a > dummy LUID. Mark that LUID as deleted in the change log. Then in the > next session, delete the redundant item on the server. > > I'm combining that with running multiple SyncML sessions in the same > context. > > Will post code soon...
The result is in the "internal-sync" branch in the meego.gitorious.org repo of libsynthesis. It's called "internal-sync" because I am working on an extension of the SyncML protocol that is only understand when SyncEvolution talks to SyncEvolution (either in the SyncEvolution local sync mode - see the http://syncevolution.org/blogs/pohly/2012/fosdem-2012 talk for a diagram illustrating that) or in SyncEvolution client to SyncEvolution server mode. Instead of repeating myself, let me quote the commit messages of the relevant commits on that branch below. There are a few more commits in that branch. Lukas, what do you think? This is not done yet, but if possible I'd like to get feedback before going down an entirely wrong path. Some tests for this new code are in SyncEvolution (not pushed yet) and pass as expected. I'll keep working on this, in particular the "temporary problem" part and "add more data in second cycle" cases. commit 37910c119381eb9e8d64a3183b4f29f2fe5b246f Author: Patrick Ohly <[email protected]> Date: Tue Feb 7 10:56:52 2012 +0100 DB_Conflict (409): different implementation in binfileimplds.cpp The previous approach to handling a 409 status for an <Add> (= item conflicts with existing item) in the binfile client was to update that existing item and then tell the server that its item maps to an existing client item. This leads to the situation where the server has two client IDs mapping to the same item on the server and one server item with no corresponding client item. Servers get confused by this. For example, the Synthesis engine itself then sends a <Delete> with empty IDs to the client in the next sync. It also has the disadvantage that the client cannot ask the server to delete the redundant item, because its <Delete> requests must include a client ID, which the redundant item doesn't have. Furthermore, the server was sent a redundant <Update> in the case that it already had an item that was in sync in the client (= item didn't have to be written in local storage). Therefore this commit implements a different approach: - the new server item which triggers the 409 is used to update the existing item, as before - if and only if the local item gets modified, it will be sent as update for the older server item already mapped to it (there must be such an item, because the client must have told the server about that local item, and that server item is now out-dated) - the new server item is mapped to a fake client ID which is marked as deleted; in the next sync, that mapping is used to delete the new server item Once a second sync is done, client and server are in sync again, with the latest data as determined by the client stored in the servers older item and the newer item deleted. Generating a fake client ID is a bit hacky at the moment. The code for numeric IDs is entirely untested, the code for string IDs doesn't check for (unlikely?!) collisions. commit 6504ae35543efe860feeda4ce498ec9824a74d3d Author: Patrick Ohly <[email protected]> Date: Tue Feb 7 15:46:29 2012 +0100 SyncML extensions: multiple cycles in the same session SyncML limits the data exchange to "client sends changes, server sends changes back". If the client detects that it has further changes for the server while already in the second phase, it will have to wait for the next SyncML session before it can send these changes. This leads to a bad user experience, because the user expectation is that client and server are in sync after a sync session. Examples when this becomes a problem: - 409 conflict detected in binfile client (tested with SyncEvolution Client::Sync::eds_event::testAddBothSides[Refresh] and a SyncEvolution server with file backend which doesn't detect duplicates based on UID) - temporary error prevents applying a change, needs to be retried (untested at the moment) - PBAB use case: advanced client does a sync with a subset of the data (vCard without PHOTO, while retrieving PHOTO data in the background), then sends updates with more data (modified/new PHOTOs) later on (also untested) This commit introduces a new mode where SyncML client and server allow multiple read/write cycles inside the same session. This extends the SyncML 1.2 standard in the following way: - in refresh-from-client mode, client and server always enter the map phase although the standard says that it can and must be skipped; the Synthesis engine already supported that non-standard behavior via the fCompleteFromClientOnly flag (aka <completefromclientonly> in the config and remote rules) - the client, if it has more changes for the server and hasn't encountered problem, finishes the current cycle internally and then sends a new <Alert> in the next message to start the next cycle, instead of ending the map phase and package with a <Final/> tag - the server starts at the beginning of a new SyncML session when it gets that <Alert>, instead of rejecting it as a protocol violation - the first sync mode is done as set by the app; any following one falls back to incremental mode: - slow sync -> two-way sync or one-way if data direction was limited - refresh sync -> one-way sync in same direction - one-way sync -> do it again To simplify testing, this new functionality is currently limited to sync sessions with only a single active data store. The new mode has to be enabled *on both sides* with the <completefromclientonly> config option. When only enabled on the client side, the unexpected <Alert> will be rejected by the serer. When only enabled on the server side, the client might be surprised by the non-standard map phase behavior. SyncEvolution enables this new mode unconditionally with a remote rule when talking to itself. Ideally this should be replaced with a DevInf extension, both for "SyncML engine supports the mode" and "data store supports restarting". Data stores must be able to deal with repeated cycles in the same session. Typically this consists of reseting some state so that running another sync cycle doesn't reuse obsolete information. The binfile DS supports this now, but the underlying data store in the app might not be ready for it yet. Currently the engine does not check this. The binfile client automatically recognizes the 409 conflict case and enters a new sync cycle to get the server in sync with the client. In addition, any client can be told to restart the sync by setting the new "restartsync" session variable (PBAP use case). This has to happen before the client considers the session completed, i.e. before EndDataWrite() is called. It would be better to do this with a per-datastore flag, but it wasn't obvious how to expose such a flag via the engine API. While looking at the code it was noticed that issueCustomEndPut() is not called when running in standard-compliant refresh-from-client mode, because the code is only invoked at the end of the map phase, which is skipped in that mode. A bug?! Not fixed. commit 1da2ea00d5b09fd0c9c091cfd2a3ae1577206516 Author: Patrick Ohly <[email protected]> Date: Tue Feb 7 15:50:36 2012 +0100 datastore: explicitly tell the engine whether restarting a sync is supported The non-standard "multiple sync cycles in the same session" mode depends on data stores which support multiple read/write cycles and properly report changes at the start of additional cycles. The engine will automatically run multiple cycles if it deems that necessary to get client and server in sync. This commit introduces the necessary flag that must be set for a store before the engine can be sure that it is safe to run more than one cycle. Not currently checked by the engine, though! -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. _______________________________________________ os-libsynthesis mailing list [email protected] http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
