Closed
Bug 1210625
Opened 10 years ago
Closed 10 years ago
meta/global modified timestamp is not updated on successful PUT
Categories
(Cloud Services Graveyard :: Server: Sync, defect)
Cloud Services Graveyard
Server: Sync
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: nalexander, Assigned: rfkelly)
References
Details
Attachments
(1 file)
After a 200 PUT to storage/meta/global, I see the 'modified' timestamp is not being updated. The 'meta' record of info/collections is being updated.
To reproduce, find a way to look at storage/meta/global and info/collections. I used a hacked up version of https://github.com/edmoz/fxa-sync-client; I intend to upstream my changes shortly.
Then, connect Desktop to a test Firefox Account. Force a sync. Observe that meta/global is present on the server, and that info/collections reflects the record's modified timestamp.
Go to Sync preferences and deselect some engines; force a sync. Observe that meta/global has been updated on the server; that meta/global's modified timestamp has *not* been updated; and that info/collections reflects a timestamp newer than the record's modified timestamp.
Reporter | ||
Comment 1•10 years ago
|
||
I observe that Android carefully uses the fetch timestamp and not the record modified field: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/stage/FetchMetaGlobalStage.java?offset=0#32
rnewman: are you aware of this inconsistency on the server? Do you have any tickets or context to add? (It appears not; your iOS code uses the record modified field, and suffers for it.)
Flags: needinfo?(rnewman)
Comment 2•10 years ago
|
||
This sounds like a server bug, and it sounds like the kind of thing that would cause bugs similar to Bug 692620.
---
X-Last-Modified
This header gives the last-modified time of the target resource as seen during processing of the request, and will be included in all success responses (200, 201, 204). When given in response to a write request, this will be equal to the server’s current time and to the new last-modified time of any BSOs created or changed by the request.
---
---
modified none float, 2 decimal places The timestamp at which this object was last modified, in seconds since UNIX epoch (1970-01-01 00:00:00 UTC). This is set automatically by the server according to its own clock; any client-supplied value for this field is ignored.
---
So the server is using the client-supplied "modified" attribute?
Severity: normal → major
Component: Firefox Sync: Backend → Server: Sync
Flags: needinfo?(rnewman) → needinfo?(rfkelly)
OS: Unspecified → All
Hardware: Unspecified → All
Comment 3•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #1)
> I observe that Android carefully uses the fetch timestamp and not the record
> modified field:
It does so instead of using the info/collections response that's already held.
I don't think this is deliberately working around this bug… and, of course, it's still incorrect when the modified stamp of the record is compared to a persisted value.
Assignee | ||
Comment 4•10 years ago
|
||
> So the server is using the client-supplied "modified" attribute?
Yes, it looks like the memcached storage engine copies over any modified stamp provided by the client:
https://github.com/mozilla-services/server-syncstorage/blob/master/syncstorage/storage/memcached.py#L690
The plain SQL storage engine does not, which would explain why we see this for meta/global but not other record types.
Assignee | ||
Comment 5•10 years ago
|
||
The memcached backend deliberately copies the client-provided modified time, but I've no idea why. I just removed the code and it hasn't broken any testcases. *shrug*
:zaach, since you're working on sync stuff these days, want to r? this relatively trivial server-side patch?
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8668892 -
Flags: review?(zack.carter)
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #3)
> (In reply to Nick Alexander :nalexander from comment #1)
> > I observe that Android carefully uses the fetch timestamp and not the record
> > modified field:
>
> It does so instead of using the info/collections response that's already
> held.
>
> I don't think this is deliberately working around this bug… and, of course,
> it's still incorrect when the modified stamp of the record is compared to a
> persisted value.
I assume you mean it /would/ be incorrect, 'cuz we don't do this in the Android Sync code. At no time do we use the modified stamp of the two records we persist (meta/global and crypto/keys). We always use the X-Weave-Timestamp of the record fetch, and compare against the corresponding entry in info/collections. That should be always safe. Do you agree?
Flags: needinfo?(rnewman)
Comment 8•10 years ago
|
||
Yes.
There are three timestamps available at that moment: the I/c we're holding, the response timestamp, and the record timestamp.
The latter two should be identical; that they are not is this bug. The comment in that code refers to not using I/c, rather than predicting this bug.
Flags: needinfo?(rnewman)
Comment 9•10 years ago
|
||
Note that the meta timestamp can be later than m/g in Sync 1.1, because of the migration sentinel. In that instance we'll do redundant work, comparing m/g by content when we should be able to discard a non-change by timestamp.
Updated•10 years ago
|
Attachment #8668892 -
Flags: review?(zack.carter) → review+
Assignee | ||
Comment 10•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•