Closed
Bug 1214189
Opened 9 years ago
Closed 9 years ago
Retry after conflict in history data adapter
Categories
(Firefox OS Graveyard :: Sync, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
FxOS-S11 (13Nov)
People
(Reporter: mbdejong, Assigned: mbdejong)
References
Details
Attachments
(1 file)
It seems to me that a DataStore.put with revisionId parameter can fail. When this happens in https://github.com/mozilla-b2g/gaia/blob/master/apps/sync/js/adapters/history.js#L127 we should probably repeat the get->merge->put cycle, until it succeeds.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mbdejong
Blocks: 1195647
Summary: Retry after conflict → Retry after conflict in history data adapter
Comment 2•9 years ago
|
||
Hey Michiel, Thanks for finding this. :) Could you paste the log message or the steps to reproduce?
Flags: needinfo?(selee)
Comment 3•9 years ago
|
||
In addition, I didn't get why DataStore.put with revisionId parameter can fail. So I am looking forward to more information. :P
Assignee | ||
Comment 4•9 years ago
|
||
I don't have steps to reproduce, just ran into this question while reading the code. The docs [1] say DataStore.put can "be aborted", so I guess this will look like a Promise rejection, but I'm not sure. Will try it out and propose a patch! [1] https://developer.mozilla.org/en-US/docs/Web/API/DataStore/put#Parameters
Assignee | ||
Comment 5•9 years ago
|
||
Same issue exists in bookmarks adapter, I'll add unit tests and fix in both.
Assignee | ||
Updated•9 years ago
|
Target Milestone: --- → FxOS-S9 (16Oct)
Updated•9 years ago
|
Target Milestone: FxOS-S9 (16Oct) → FxOS-S11 (13Nov)
Assignee | ||
Comment 6•9 years ago
|
||
An easy way to deal with DataStore race conditions, is to reject the sync run altogether, see bug 1215470.
Updated•9 years ago
|
Priority: -- → P3
Comment 7•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8684211 -
Flags: review?(ferjmoreno)
Comment 8•9 years ago
|
||
Comment on attachment 8684211 [details] [review] [gaia] michielbdejong:1214189-datastore-conflicts > mozilla-b2g:master LGTM, thanks. It seems that you forgot to add the fix for bookmarks as you mentioned in comment 5. Do you still want to do it in this bug or do you want to do it in a follow up?
Attachment #8684211 -
Flags: review?(ferjmoreno) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Oh right, no, better to do it now for bookmarks as well. I'll add it within this bug and re-submit.
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8684211 [details] [review] [gaia] michielbdejong:1214189-datastore-conflicts > mozilla-b2g:master Made same change for bookmarks as well.
Attachment #8684211 -
Flags: review+ → review?(ferjmoreno)
Comment 12•9 years ago
|
||
Comment on attachment 8684211 [details] [review] [gaia] michielbdejong:1214189-datastore-conflicts > mozilla-b2g:master Thank you Michiel. LGTM, but there is a failing test. Also, could you file a bug to move the common logic between the history and bookmarks adapter to a shared script, please?
Attachment #8684211 -
Flags: review?(ferjmoreno) → review+
Assignee | ||
Comment 13•9 years ago
|
||
Thanks! Failing tests fixed with https://github.com/mozilla-b2g/gaia/pull/33043/files#diff-6dac2f6d58d68fbc11a10804f9946d4fR178 (still waiting for TreeHerder). Follow-up is bug 1224183.
Flags: needinfo?(ferjmoreno)
Assignee | ||
Comment 15•9 years ago
|
||
Ah sorry, rebased (waiting for TreeHerder again).
Flags: needinfo?(mbdejong) → needinfo?(ferjmoreno)
Comment 16•9 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/ee5d88665fa0f6b02afd13173f5dfa3459dee2fa
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(ferjmoreno)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•