Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify specifications about replication / forcing timestamps in records #1362

Open
leplatrem opened this issue Nov 7, 2017 · 4 comments
Open
Labels
documentation protocol question scope: core stale For marking issues as stale. Labeled issues will be closed soon if label is not removed.

Comments

@leplatrem
Copy link
Contributor

In the docs, we can read:

«If [the specified timestamp] is in the past, the record is created with the timestamp in the past but the list timestamp is bumped into the future as usual.» source

While trying to fix #1357 I realize I wasn't able to remove the bottleneck and obtain this behaviour. Good news is that the behaviour specified in the tests suite are consistent with the docs.

But I can't recall how and why that makes sense (apparently @Natim either). It comes from mozilla-services/cliquet#665 at the time @almet was working on kinto-signer replication.

Can we explain somewhere how the synchronization will work in that case? How will the client poll for changes and obtain the records in the past if we only bump the collection timestamp?

If we clarify this, there is a chance we can fix #1357 with something like in #1361

The tests are:

  • test_create_ignores_specified_last_modified_if_in_the_past
    def test_create_ignores_specified_last_modified_if_in_the_past(self):
    # Create a first record, and get the timestamp.
    first_record = self.create_record()
    timestamp_before = first_record[self.modified_field]
    # Create a new record with its timestamp in the past.
    record = {**self.record,
    self.id_field: RECORD_ID,
    self.modified_field: timestamp_before - 10}
    self.create_record(record=record)
    # Check that record timestamp is the one specified.
    retrieved = self.storage.get(object_id=RECORD_ID, **self.storage_kw)
    self.assertLess(retrieved[self.modified_field], timestamp_before)
    self.assertEquals(retrieved[self.modified_field],
    record[self.modified_field])
    # Check that collection timestamp was bumped (change happened).
    timestamp = self.storage.collection_timestamp(**self.storage_kw)
    self.assertGreater(timestamp, timestamp_before)
  • test_update_ignores_specified_last_modified_if_in_the_past
    def test_update_ignores_specified_last_modified_if_in_the_past(self):
    stored = self.create_record()
    record_id = stored[self.id_field]
    timestamp_before = stored[self.modified_field]
    # Set timestamp manually in the future.
    stored[self.modified_field] = timestamp_before - 10
    self.storage.update(object_id=record_id, record=stored,
    **self.storage_kw)
    # Check that record timestamp is the one specified.
    retrieved = self.storage.get(object_id=record_id, **self.storage_kw)
    self.assertLess(retrieved[self.modified_field], timestamp_before)
    self.assertEquals(retrieved[self.modified_field],
    stored[self.modified_field])
    # Check that collection timestamp was bumped (change happened).
    timestamp = self.storage.collection_timestamp(**self.storage_kw)
    self.assertGreater(timestamp, timestamp_before)
@almet
Copy link
Member

almet commented Nov 7, 2017

Hi Kinto team :-) It's cool to see that things are still moving around here! It's been a long time since I commented on anything, I hope I'm able to help you out.

From what I recall, at some point the kinto-signer was copying records from one collection to another (to update the destination collection records with the recent changes), and it was needed to not modify their last_modified field (because otherwise the serialization would differ, and thus the signature not match).

At the same time, I believe it was needed to bump the collection in order to make it clear that there were a change in the records inside the collection.

Hope I'm not too far away from the current question, as I didn't took the time to understand the full context around the discussion.

@almet
Copy link
Member

almet commented Nov 7, 2017

I believe this is because of this line where we get the records that were updated since the latest signature. If we do not update the timestamp of the collection, then we would also get old records?

@leplatrem
Copy link
Contributor Author

Yes, now that you mention that, it could be that... We want to keep the records unchanged when copying them, but bump the collection timestamp of records (for kinto-changes etc).

But honestly, I can't see why it should be like that, especially because I believe the signature is computed on the final records set. And since we copy the records once the staging collection has been edited, the timestamps on the destination will always be superior :/

And more generally, the kinto-signer thing should not come in the way :) If we look at it as it is, in terms of synchronization, the current code/docs don't seem to make sense :(

@glasserc
Copy link
Contributor

glasserc commented Nov 9, 2017

Thanks for digging into this so much @leplatrem, and thanks for the explanation @almet. I don't claim to 100% understand the factors in play here, but I agree that bumping a collection timestamp with record timestamps in the past doesn't seem to make sense, because follow-up queries with ?_since=... won't match the new records and will generally just confuse the client. (This would probably give you a Server was flushed in kinto.js.) I understand how exact replication of timestamps could be important for kinto-signer use cases, but I don't understand how it could possibly be correct to bump a timestamp while preserving a different one.

@alexcottner alexcottner added the stale For marking issues as stale. Labeled issues will be closed soon if label is not removed. label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation protocol question scope: core stale For marking issues as stale. Labeled issues will be closed soon if label is not removed.
Projects
None yet
Development

No branches or pull requests

4 participants