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.
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.)
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
(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.
> 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: nobody → rfkelly
No longer blocks: 1167010
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?
Created attachment 8668892 [details] [review] remove buggy and apparently-point code that copies the client timestamp
Attachment #8668892 - Flags: review?(zack.carter)
(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?
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.
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.
Attachment #8668892 - Flags: review?(zack.carter) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.