Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
Hi Patrick, On Mar 9, 2012, at 14:30 , Patrick Ohly wrote: > On Tue, 2012-03-06 at 14:50 +0100, Patrick Ohly wrote: >> I haven't look into this yet, but still have it on my radar. > > Done in the meego.gitorious.org master branch. I found that checking for > collisions is hard (not all records are in memory), so I settled for > making the chance of collisions smaller in the string case by including > a running count. I guess this is way safe enough. The worst that can happen is that two (at that time by definition already obsolete) server items will get a mapping to the same client ID when a fake-ID generation collision should occur (which now can only happen with suspend&resume where libsynthesis is reinstantiated in between). If so, the client will send two deletes for the same clientID to the server in a subsequent sync. Depending on the server implementation, either the first delete wipes all items mapped to that ID and the second delete will get a 404, which is fine. Or the first delete wipes just the first item that maps to that item, and the second delete then wipes the second - correct as well. Even if a super-smart server would merge the two items upon receiving a map to the same clientID, the end result would be correct (altough the merged item would likely make no sense - but as it is doomed at the time of merge already that would be an acceptable intermediate state for a case that is extremely unlikely to occur at all). Best Regards, Lukas Lukas Zeller, plan44.ch l...@plan44.ch - www.plan44.ch ___ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
On Tue, 2012-03-06 at 14:50 +0100, Patrick Ohly wrote: > I haven't look into this yet, but still have it on my radar. Done in the meego.gitorious.org master branch. I found that checking for collisions is hard (not all records are in memory), so I settled for making the chance of collisions smaller in the string case by including a running count. -- 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 os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
On Thu, 2012-02-16 at 18:14 +0100, Lukas Zeller wrote: > On Feb 13, 2012, at 14:35 , Patrick Ohly wrote: > > > 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? > > > Thanks for these patches (the one with firstReadNextItem not being set > is an embarassing one - on the other hand it proves safe coding in all > of the plugins in use so far in that they apparently all cleanly reset > the iterator at initialisation without being told so). I guess they would have failed in very obvious ways if they hadn't done that. My main concern was that they now might now start to fail because the engine works as described ;-) > Regarding the solution for the 409 merge problem, I think the > fake-ID-delete approach is fine. Numeric localids are not used in any > libsynthesis build, this is legacy from the monolithic PalmOS clients > which had severe memory constraints which ruled out anything as fat > and resource wasting as string IDs :-) Still, I think the use of > negative IDs would work here, the only thing I'm not sure is if > RND_MAX can be safely assumed to be 31 bits only - in case it would > extend into 32 bit we'd get positive IDs clashing with real ones > occasionally. Maybe just AND rand() result with 0x7FFF to be on > the safe side... I haven't look into this yet, but still have it on my radar. Instead I continued investigating multi-cycle syncs by writing more tests and fixing some of the other TODO items. All of that works now, so I merged the code into the master branched of the meego.gitorious.org repos. -- 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 os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
Hi Patrick, On Feb 13, 2012, at 14:35 , Patrick Ohly wrote: > 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? Thanks for these patches (the one with firstReadNextItem not being set is an embarassing one - on the other hand it proves safe coding in all of the plugins in use so far in that they apparently all cleanly reset the iterator at initialisation without being told so). Regarding the solution for the 409 merge problem, I think the fake-ID-delete approach is fine. Numeric localids are not used in any libsynthesis build, this is legacy from the monolithic PalmOS clients which had severe memory constraints which ruled out anything as fat and resource wasting as string IDs :-) Still, I think the use of negative IDs would work here, the only thing I'm not sure is if RND_MAX can be safely assumed to be 31 bits only - in case it would extend into 32 bit we'd get positive IDs clashing with real ones occasionally. Maybe just AND rand() result with 0x7FFF to be on the safe side... Best Regards, Lukas ___ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
On Tue, 2012-02-07 at 16:05 +0100, Patrick Ohly wrote: > 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. I've also pushed the "internal-sync" branch for SyncEvolution. It has tests for the "409 in client of dumb SyncML server" problem (testAddBothSides when configured to use SyncEvolution server with file backend), restarting a two-way sync with SyncContext::requestAnotherSync() (testTwoWayRestart) and the extended SyncSource semantic. All of that works fine. It took a bit longer than expected because writing these tests required quite a bit of code refactoring. Speaking of the extended semantic that beginSync() may be called multiple times: right now this is mandatory. My hope is that backend implementers will adapt to that change before releasing 1.3, or that no such changes are necessary. EDS and file backends worked fine without changes. I prefer that approach because it avoids special cases. Mikel, most relevant for you is the new SyncSource/SyncContext::requestAnotherSync() API call. You might be able to implement the two-phase transfer of contacts like this: 1. In first beginSync() call: * only retrieve contacts without PHOTO data * ask for another cycle with requestAnotherSync() 2. In the second beginSync(): * also retrieve PHOTO data * mark all contacts with new or updated PHOTO data as "updated" I also implemented the "backend is read-only" part. The Synthesis engine already had that, it was just a matter of making that available in SyncEvolution. You can rebase the PBAP branch onto "internal-sync". I'll refrain from rebasing "internal-sync" from now on. -- 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 os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] 409 "item merged" in client + multiple sync cycles in a single session
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 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 (= 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 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 requests must include a client ID, which the redundant item doesn't have. Furthermore, the server was sent a redundant 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 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: - i
