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 <patrick.ohly(a)intel.com>
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 <patrick.ohly(a)intel.com>
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 <patrick.ohly(a)intel.com>
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.