Lukas Zeller | 8 Sep 2011 00:44
Picon
Gravatar

Re: UID matching during add<->add conflict

Hi Patrick,

at least I could do one half of the pending work yesternight... 

On Aug 29, 2011, at 17:00 , Patrick Ohly wrote:

> [...]
> I've written such a test and found some problems:
>     1. compile issue
>     2. TPluginApiDS::apiAddItem() treated DB_DataMerged as a failure
>        and didn't return the local ID to
>        TCustomImplDS::implProcessItem(), which caused a segfault later
>        on (local ID NULL)
> 
> Patches attached.

Thanks.

> But now I still see an unexpected Replace command.

I looked up what's happening in the code and the replace is not unexpected to me:

> Here's the processing
> of the client's Add in a SyncEvolution server where that Add maps to an
> item that was new for this client:
> 
> [2011-08-29 16:43:57.474] adding "meeting on site, big meeting room"
> [2011-08-29 16:43:57.479] InsertItemAsKey res=207
> [2011-08-29 16:43:57.480] Database adapter indicates that added item was merged with pre-existing
data (status 207)
(Continue reading)

Patrick Ohly | 8 Sep 2011 08:10
Picon
Favicon
Gravatar

Re: UID matching during add<->add conflict

On Do, 2011-09-08 at 00:44 +0200, Lukas Zeller wrote:
> On Aug 29, 2011, at 17:00 , Patrick Ohly wrote:
> > [...]
> > I've written such a test and found some problems:
> >     1. compile issue
> >     2. TPluginApiDS::apiAddItem() treated DB_DataMerged as a failure
> >        and didn't return the local ID to
> >        TCustomImplDS::implProcessItem(), which caused a segfault later
> >        on (local ID NULL)
> > 
> > Patches attached.
> 
> Thanks.
> 
> > But now I still see an unexpected Replace command.
> 
> I looked up what's happening in the code and the replace is not unexpected to me:
[...]
> Status 207 is meant to indicate that the backend has added the item,
> but in the process merged the add with some additional data (from an
> external source, or another item in the sync set). So...

What the backend really did was replace its own data entirely with the
data sent to it, with no merging involved at all. Therefore sending back
that same data is unnecessary and could be avoided.

What we would need is another status "replaced existing item". Either
that, or allow the case that an Add returns 200 with a LUID that was
already in use before. The explicit status for it might be easier to
check for the engine.
(Continue reading)

Lukas Zeller | 8 Sep 2011 09:59
Picon
Gravatar

Re: UID matching during add<->add conflict

Hello Patrick,

On Sep 8, 2011, at 8:10 , Patrick Ohly wrote:

>> Status 207 is meant to indicate that the backend has added the item,
>> but in the process merged the add with some additional data (from an
>> external source, or another item in the sync set). So...
> 
> What the backend really did was replace its own data entirely with the
> data sent to it, with no merging involved at all. Therefore sending back
> that same data is unnecessary and could be avoided.
> 
> What we would need is another status "replaced existing item".

Yes. After writing that mail, I realized that there could be cases where the backend detects a *perfect*
duplicate or chooses for other reasons to replace its own data with the data sent with no field level
merging (nor need for it).

I'll add another status for that.

> [...] Either that, or allow the case that an Add returns 200 with a LUID that was
> already in use before. The explicit status for it might be easier to
> check for the engine.

And more efficient in all non-conflict cases.

>> No, that's the server updating the client with the result of the merge.
>> 
>> Why would you expect this to be suppressed? On the contrary, this replace is explicitly forced by the 207
status handling code.
(Continue reading)

Lukas Zeller | 8 Sep 2011 16:55
Picon
Gravatar

Re: UID matching during add<->add conflict

Hello Patrick,

On Sep 8, 2011, at 9:59 , Lukas Zeller wrote:

> PS: I'm in the code now :-)

Done, but not tested beyond not interfering with normal operation in my context.

Here's the patch, I'm currently shuffling my repos around so I haven't pushed it yet.

Best Regards,

Lukas

---- the patch ----

From 25f7b2bf8187544cf357a13214f65b2902f14bc8 Mon Sep 17 00:00:00 2001
From: Lukas Zeller <luz@...>
Date: Thu, 8 Sep 2011 16:41:44 +0200
Subject: [PATCH] engine: added more merge options for DB implementations to
 ask engine for.

Up to now, only DB_DataMerged (207) had a special meaning when
returned by the DB backend for a server add.

Now the following options exist for server add operations:

- DB backend returns DB_DataMerged (207):

  This means that the backend has augmented the item
(Continue reading)

Lukas Zeller | 10 Sep 2011 12:32
Picon
Gravatar

Re: StartDataRead/EndDataWrite + invalid anchor

Hello Patrick,

On Sep 7, 2011, at 14:53 , Patrick Ohly wrote:

> SyncEvolution's ActiveSync backend uses the sync anchor passed into
> StartDataRead() for change tracking. The new anchor is created in
> EndDataWrite(). The sync anchor is the ActiveSync "sync key", created
> and updated by the ActiveSync server as changes are made to the data.
> 
> I now have the following situation: because of a misconfiguration of the
> client, it attempts to start a sync with an invalid sync key. The
> ActiveSync server rejects it in StartDataRead(), which returns a 10500
> error code.
> 
> [...]
> 
> The error causes apiReadSyncSet() to bail out, without ever calling
> EndDataWrite() in that session. Because the backend never gets the
> chance to reset the sync anchor, the next sessions just do the same
> thing again and again.

Ok, I see the problem.

StartDataRead() is unfortunately too late in the process of the SyncML protocol to convert the sync into a
slow sync in the same session (latest point where this is possible is when handling the Alert from the
server, which is before starting to read from the DB).

But what we could do is allow StartDataRead to return 508 error code to have the engine reset sync anchors
before bailing out, such that the next session would become a slow sync.

(Continue reading)

Andris Pavenis | 12 Sep 2011 11:59
Picon

Contact Synchronization problem between libsynthesis based server and S60 3rd Edition phones

There are following problems synchronizing contacts between libsynthesis
based server (our own implementation) and S60 3rd edition mobile phones
(I used Nokia E65 and E50 for testing). I'm giving an example to illustrate
the problem:

- let us assume that contact on both server and in mobile phone address book
  contain one or several phone numbers of the same type (for example
  TEL_FLAGS[xx]:8):
       TEL[0]:11111111
       TEL_FLAGS[0]:8
       TEL[1]:22222222
       TEL_FLAGS[1]:8
       TEL[2]:33333333
       TEL_FLAGS[2]:8
       TEL[3]:44444444
       TEL_FLAGS[3]:8
 
- a part of phone numbers are deleted on server side (not from the end):
       TEL[0]:33333333
       TEL_FLAGS[0]:8
       TEL[1]:44444444
       TEL_FLAGS[1]:8

- An attempt to synchronize device causes SyncML Replace command
  to be generated for updating contact on client side. First 2 numbers
  on client device are replaced with 2 numbers send from server
  but 3rd and 4th number remains unchanged
       TEL[0]:33333333
       TEL_FLAGS[0]:8
       TEL[1]:44444444
       TEL_FLAGS[1]:8
       TEL[2]:33333333
       TEL_FLAGS[2]:8
       TEL[3]:44444444
       TEL_FLAGS[3]:8

The same problem appears also when there is only one number of each type
present and some of them are deleted on server side. After synchronization
it is still present on client side (none replaced, so the first remains where it was)

As far as I checked Funambol server tries to workaround this problem by sending
empty phone number after I remove phone number using Funambol WebGui demo
version (it supports however only one number of each type so test possibilities are
rather limited)

I also saw a different bad behavior of synchronization in case of S60 3rd edition,
feature pack 1 (I tested with Nokia E90 Communicator). In this case as the result
of the Replace command only the last number of each type appears on device
(all other numbers disappear)

Is there any possibility to workaround such problems?

Andris

_______________________________________________
os-libsynthesis mailing list
os-libsynthesis@...
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Patrick Ohly | 12 Sep 2011 14:16
Picon
Favicon
Gravatar

Re: StartDataRead/EndDataWrite + invalid anchor

On Sa, 2011-09-10 at 12:32 +0200, Lukas Zeller wrote:
> Hello Patrick,
> 
> On Sep 7, 2011, at 14:53 , Patrick Ohly wrote:
> 
> > SyncEvolution's ActiveSync backend uses the sync anchor passed into
> > StartDataRead() for change tracking. The new anchor is created in
> > EndDataWrite(). The sync anchor is the ActiveSync "sync key", created
> > and updated by the ActiveSync server as changes are made to the data.
> > 
> > I now have the following situation: because of a misconfiguration of the
> > client, it attempts to start a sync with an invalid sync key. The
> > ActiveSync server rejects it in StartDataRead(), which returns a 10500
> > error code.
> > 
> > [...]
> > 
> > The error causes apiReadSyncSet() to bail out, without ever calling
> > EndDataWrite() in that session. Because the backend never gets the
> > chance to reset the sync anchor, the next sessions just do the same
> > thing again and again.
> 
> Ok, I see the problem.
> 
> StartDataRead() is unfortunately too late in the process of the SyncML
> protocol to convert the sync into a slow sync in the same session
> (latest point where this is possible is when handling the Alert from
> the server, which is before starting to read from the DB).
> 
> But what we could do is allow StartDataRead to return 508 error code
> to have the engine reset sync anchors before bailing out, such that
> the next session would become a slow sync.
> 
> Would that solve the problem?

It is sub-optimal because it will be visible to users. Currently
SyncEvolution doesn't automatically start a new sync. This will also
break the automated testing, which will bail out with that error instead
of doing a slow sync.

As one way of dealing with the problem adding such a special error code
is worthwhile. But perhaps there is a different way?

For example, could the datastore API extended with a "verify anchor"
call which is called soon enough to discard the anchor and fall back to
a slow sync?

--

-- 
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.
Andris Pavenis | 13 Sep 2011 13:03
Picon

Re: Contact Synchronization problem between libsynthesis based server and S60 3rd Edition phones

On 09/12/2011 12:59 PM, Andris Pavenis wrote:
There are following problems synchronizing contacts between libsynthesis
based server (our own implementation) and S60 3rd edition mobile phones
(I used Nokia E65 and E50 for testing). I'm giving an example to illustrate
the problem:

- let us assume that contact on both server and in mobile phone address book
  contain one or several phone numbers of the same type (for example
  TEL_FLAGS[xx]:8):
       TEL[0]:11111111
       TEL_FLAGS[0]:8
       TEL[1]:22222222
       TEL_FLAGS[1]:8
       TEL[2]:33333333
       TEL_FLAGS[2]:8
       TEL[3]:44444444
       TEL_FLAGS[3]:8
 
- a part of phone numbers are deleted on server side (not from the end):
       TEL[0]:33333333
       TEL_FLAGS[0]:8
       TEL[1]:44444444
       TEL_FLAGS[1]:8

- An attempt to synchronize device causes SyncML Replace command
  to be generated for updating contact on client side. First 2 numbers
  on client device are replaced with 2 numbers send from server
  but 3rd and 4th number remains unchanged
       TEL[0]:33333333
       TEL_FLAGS[0]:8
       TEL[1]:44444444
       TEL_FLAGS[1]:8
       TEL[2]:33333333
       TEL_FLAGS[2]:8
       TEL[3]:44444444
       TEL_FLAGS[3]:8

The same problem appears also when there is only one number of each type
present and some of them are deleted on server side. After synchronization
it is still present on client side (none replaced, so the first remains where it was)

As far as I checked Funambol server tries to workaround this problem by sending
empty phone number after I remove phone number using Funambol WebGui demo
version (it supports however only one number of each type so test possibilities are
rather limited)
Found that also with Funambol SyncML server (tested on Linux x86_64 with unmodified
package from Sourceforge) I have the same problem when synchronizing Nokia E65 when
tested synchronization:

ZTE Blade (Android) ==> Funambol server ==> Nokia E65

The testing was more difficult as Funambol server has it own restrictions and problems.

So the problem with removing phone numbers and email addresses from contacts
on S60 3rd edition phones is not only libsynthesis specific.

Andris
 
_______________________________________________
os-libsynthesis mailing list
os-libsynthesis@...
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Patrick Ohly | 15 Sep 2011 09:32
Picon
Favicon
Gravatar

Re: UID matching during add<->add conflict

Hello!

There's one more minor issue: in server mode, when an Add command
results in modifying an existing item, the statistics say that the item
was added. That means that the numbers do not add up, because "items
before sync + added items != items after sync".

I'm seeing it with DB_Conflict, but I think it'll also occur with
DB_DataReplaced/Merged.

The point where the statistics are changes is in
TLocalEngineDS::engProcessRemoteItemAsServer():

        // add allowed
===>    fLocalItemsAdded++;
        #ifdef OBJECT_FILTERING
        // test if acceptable
        if (!isAcceptable(aSyncItemP,aStatusCommand)) { ok=false; break; } // cannot be accepted
        // Note: making item to pass sync set filter is implemented in derived DB implementation
        //   as criteria for passing might be in data that must first be read from the DB
        #endif
        remainsvisible=true; // should remain visible
        ok=logicProcessRemoteItem(aSyncItemP,aStatusCommand,remainsvisible); // add to local database NOW

What would be the right fix for this? Add another retval to
logicProcessRemoteItem() which tells the caller whether an add was
turned into an update? Or change the return value from bool to an enum?

--

-- 
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.

Gmane