11:45 a.m.
http://bugs.meego.com/show_bug.cgi?id=11044
Summary: temporary UID mapping
Classification: MeeGo Projects
Product: SyncEvolution
Version: unspecified
Platform: All
Architecture: ---
Status: NEW
Severity: major
Priority: High
Component: SyncML
AssignedTo: syncevolution-bugs(a)meego.bugs
ReportedBy: patrick.ohly(a)intel.com
QAContact: jingke.zhang(a)intel.com
CC: syncevolution-bugs(a)meego.bugs,
syncevolution-syncml-bugs(a)meego.bugs
Estimated Hours: 0.0
This possibly explains random problems when syncing with phones which have a
short maximum ID size. Not entirely sure (no logs of such failures).
------------------------------------------------
From: Patrick Ohly <patrick.ohly(a)intel.com>
To: Lukas Zeller <luz(a)synthesis.ch>
Cc: Synthesis <os-libsynthesis(a)synthesis.ch>
Subject: [os-libsynthesis] temporary UID mapping
Date: Wed, 8 Dec 2010 11:22:22 +0000 (08.12.2010 12:22:22)
Hello!
The Synthesis engine has the feature that its backends are allowed to
use IDs of arbitrary length. The engine will translate into IDs shorter
than the maximum ID size supported by the peer.
TLocalEngineDS::adjustLocalIDforSize() creates these temporary IDs,
using:
fTempGUIDMap.size() + 1
// as list only grows, we have unique tempuids for sure
I'm currently (involuntarily ;-) stress-testing this code by running
SyncEvolution<->SyncEvolution syncs with lots of iCalendar 2.0 items,
which happen to have very long IDs.
I see failures where the server assigns the same temporary ID to
different items in the same sync.
I've added debug logging. It shows that the following happens:
1. fTempGUIDMap is restored from the map file such that it has 105
entries, *but* these are non-contiguous (from #1 to #124).
2. fTempGUIDMap.size() + 1 is #106, which already exists in
fTempGUIDMap.
3. Overwriting that entry does not increase fTempGUIDMap.size().
4. A second item is assigned the same #106 temp ID.
I don't know exactly how I arrived at this state. Is the on-disk dump of
fTempGUIDMap really meant to preserve all temporary IDs in perpetuity? I
don't think so, because that would fill up the disk and there is a
"delete map item" operation.
Unless there is some additional constraint, then the assumption that
"the list only grows" is wrong. I have added a workaround which checks
for ID collisions in TLocalEngineDS::adjustLocalIDforSize(). Is that the
right solution or do I need to search for the reason why the mapping has
gaps?
Also note that I end up with a temporary ID which must have been used
before in an older sync session. Could that be a problem?
--------------------------------------------------------
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
1:38 p.m.
New subject: [MeeGo Projects - Bug 11044] temporary UID mapping
http://bugs.meego.com/show_bug.cgi?id=11044
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |WAITING FOR UPSTREAM
AssignedTo|syncevolution-bugs(a)meego.bu |patrick.ohly(a)intel.com
|gs |
Target Milestone|--- |1.1.1
--- Comment #1 from pohly <patrick.ohly(a)intel.com> 2010-12-13 05:38:44 PST ---
I ended up patching libsynthesis to work around this issue. Now waiting for
confirmation ("[os-libsynthesis] temporary UID mapping") that these patches
work
as expected:
commit a98cdd53f73efae38352039f1d0b5798fd0aa45c
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Fri Dec 10 14:50:06 2010 +0100
maximum UID size: avoid redundant tempUID->localUID mappings
The storage of map items maps from (local id, ident) to ([remote id],
flags) and thus cannot store multiple remote ids for the same local
id. fTempGUIDMap ended up having such duplicate entries if:
- there was an entry from a previous updating of item on the client
- the item was deleted on the server, creating a new temporary id
to report deletion to the client
Such a fTempGUIDMap cannot be stored and retrieved again without
loosing the older temporary mappings, which a) might have been still
needed (not sure) and b) violated the assumption that temporary ids
have to be sequential.
This patch checks for an older mapping the the same local id and
reuses that mapping instead of creating a new one. It uses a simple
linear search, because that is all that can be done unless a more
complex data structure is introduced (which might not be worth the
effort).
commit 50026da88763db4af4077b7157f014e5472f4bab
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Wed Dec 8 12:33:58 2010 +0100
temporary IDs: avoid reusing existing ID
I see failures where the server assigns the same temporary ID to
different items in the same sync.
I've added debug logging. It shows that the following happens:
1. fTempGUIDMap is restored from the map file such that it has 105
entries, *but* these are non-contiguous (from #1 to #124).
2. fTempGUIDMap.size() + 1 is #106, which already exists in
fTempGUIDMap.
3. Overwriting that entry does not increase fTempGUIDMap.size().
4. A second item is assigned the same #106 temp ID.
I don't know exactly how I arrived at this state and whether it is valid.
This patch works around the problem by checking for an ID conflict and
iterating until it finds a truely unused ID, instead of assuming that
"hash size + 1" yields a new ID.
The debug logging is also part of the patch, in case that further
analysis is needed.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
10:42 a.m.
New subject: [MeeGo Projects - Bug 11044] temporary UID mapping
http://bugs.meego.com/show_bug.cgi?id=11044
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|WAITING FOR UPSTREAM |RESOLVED
Resolution| |FIXED
--- Comment #2 from pohly <patrick.ohly(a)intel.com> 2010-12-20 02:42:15 PST ---
Root cause found and included in 1.1.1:
commit 31f90cff28d24a5f31579039597ccfc5a534288a
Author: Lukas Zeller <luz(a)plan44.ch>
Date: Thu Dec 16 18:32:02 2010 +0100
engine: server case: fixed bad bug that could mess up tempGUIDs. These must
be cleared when first <Sync> is recei
Until now, the tempGUIDs were cleared only once when <Alert> was received.
This is ok to create a clean starting point for loading previous session's
tempGUIDs, which might be needed to resolve "early maps" also related to
the previous session.
However, as soon as the current session starts <Sync>ing, that is,
exchanging new items, these temporary IDs become invalid and
fTempGUIDMap must be cleared once again.
The missing cleanup has led to collisions in the tempGUID name space,
because the tempGUIDs are created based on the fact that the list
is empty at the beginning of <Sync>. As it could have entries
from (possibly much!) earlier sessions, all sort of weird things
could and did happen.
Thanks Patrick for the extensive investigation of this!
commit d68c46d02710c19e4db2c854966aec972542a2a9
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Sun Dec 19 22:43:51 2010 +0100
temporary ID mapping: flag unexpected state as real errors
As explained by Lukas in the "[os-libsynthesis] temporary UID mapping"
mail thread, the code which catches ID conflicts just works around
symptoms. The code should never be triggered.
This patch thus turns the messages into errors. Lukas' patch will fix
the real issue. Keeping the code is still useful to have a fallback,
just in case. At least for a while: the duplicate check is costly and
should be removed once the fix has been verified sufficiently.
--
Configure bugmail: http://bugs.meego.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
4206
days inactive
4218
days old
syncevolution-issues@syncevolution.org
2 comments
1 participants
participants (1)
-
bugzilla＠meego.com