From 5aa687b845234988b7c383a3d25fcca8e7245852 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 30 Mar 2020 12:47:54 +0200 Subject: [PATCH 1/4] Fix resource timestamp unicity (fixes #2472, #602) --- kinto/core/storage/postgresql/__init__.py | 2 +- .../migrations/migration_021_022.sql | 56 +++++++++++++++++++ kinto/core/storage/postgresql/schema.sql | 16 +++--- kinto/core/storage/testing.py | 3 +- 4 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 kinto/core/storage/postgresql/migrations/migration_021_022.sql diff --git a/kinto/core/storage/postgresql/__init__.py b/kinto/core/storage/postgresql/__init__.py index 283c4c867..1dd4a5633 100644 --- a/kinto/core/storage/postgresql/__init__.py +++ b/kinto/core/storage/postgresql/__init__.py @@ -77,7 +77,7 @@ class Storage(StorageBase, MigratorMixin): # MigratorMixin attributes. name = "storage" - schema_version = 21 + schema_version = 22 schema_file = os.path.join(HERE, "schema.sql") migrations_directory = os.path.join(HERE, "migrations") diff --git a/kinto/core/storage/postgresql/migrations/migration_021_022.sql b/kinto/core/storage/postgresql/migrations/migration_021_022.sql new file mode 100644 index 000000000..b7599f360 --- /dev/null +++ b/kinto/core/storage/postgresql/migrations/migration_021_022.sql @@ -0,0 +1,56 @@ +CREATE OR REPLACE FUNCTION bump_timestamp() +RETURNS trigger AS $$ +DECLARE + previous BIGINT; + current BIGINT; +BEGIN + previous := NULL; + WITH existing_timestamps AS ( + -- Timestamp of latest record. + ( + SELECT last_modified + FROM objects + WHERE parent_id = NEW.parent_id + AND resource_name = NEW.resource_name + ORDER BY last_modified DESC + LIMIT 1 + ) + -- Timestamp when resource was empty. + UNION + ( + SELECT last_modified + FROM timestamps + WHERE parent_id = NEW.parent_id + AND resource_name = NEW.resource_name + ) + ) + SELECT as_epoch(MAX(last_modified)) INTO previous + FROM existing_timestamps; + + -- + -- This bumps the current timestamp to 1 msec in the future if the previous + -- timestamp is equal to the current one (or higher if was bumped already). + -- + -- If a bunch of requests from the same user on the same resource + -- arrive in the same millisecond, the unicity constraint can raise + -- an error (operation is cancelled). + -- See https://github.com/mozilla-services/cliquet/issues/25 + -- + current := as_epoch(clock_timestamp()::TIMESTAMP); + IF previous IS NOT NULL AND previous >= current THEN + current := previous + 1; + END IF; + + IF NEW.last_modified IS NULL OR + (previous IS NOT NULL AND as_epoch(NEW.last_modified) = previous) THEN + -- If record does not carry last-modified, or if the one specified + -- is equal to previous, assign it to current (i.e. bump it). + NEW.last_modified := from_epoch(current); + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Bump storage schema version. +INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '22'); diff --git a/kinto/core/storage/postgresql/schema.sql b/kinto/core/storage/postgresql/schema.sql index 47030838d..65d5c4d2f 100644 --- a/kinto/core/storage/postgresql/schema.sql +++ b/kinto/core/storage/postgresql/schema.sql @@ -64,8 +64,8 @@ DROP TRIGGER IF EXISTS tgr_objects_last_modified ON objects; CREATE OR REPLACE FUNCTION bump_timestamp() RETURNS trigger AS $$ DECLARE - previous TIMESTAMP; - current TIMESTAMP; + previous BIGINT; + current BIGINT; BEGIN previous := NULL; WITH existing_timestamps AS ( @@ -87,7 +87,7 @@ BEGIN AND resource_name = NEW.resource_name ) ) - SELECT MAX(last_modified) INTO previous + SELECT as_epoch(MAX(last_modified)) INTO previous FROM existing_timestamps; -- @@ -99,16 +99,16 @@ BEGIN -- an error (operation is cancelled). -- See https://github.com/mozilla-services/cliquet/issues/25 -- - current := clock_timestamp(); + current := as_epoch(clock_timestamp()::TIMESTAMP); IF previous IS NOT NULL AND previous >= current THEN - current := previous + INTERVAL '1 milliseconds'; + current := previous + 1; END IF; IF NEW.last_modified IS NULL OR - (previous IS NOT NULL AND as_epoch(NEW.last_modified) = as_epoch(previous)) THEN + (previous IS NOT NULL AND as_epoch(NEW.last_modified) = previous) THEN -- If record does not carry last-modified, or if the one specified -- is equal to previous, assign it to current (i.e. bump it). - NEW.last_modified := current; + NEW.last_modified := from_epoch(current); END IF; RETURN NEW; @@ -131,4 +131,4 @@ INSERT INTO metadata (name, value) VALUES ('created_at', NOW()::TEXT); -- Set storage schema version. -- Should match ``kinto.core.storage.postgresql.PostgreSQL.schema_version`` -INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '21'); +INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '22'); diff --git a/kinto/core/storage/testing.py b/kinto/core/storage/testing.py index 76622eb47..23f7f21b1 100644 --- a/kinto/core/storage/testing.py +++ b/kinto/core/storage/testing.py @@ -5,7 +5,7 @@ from kinto.core import utils from kinto.core.storage import MISSING, Filter, Sort, exceptions, heartbeat -from kinto.core.testing import DummyRequest, ThreadMixin, skip_if_travis +from kinto.core.testing import DummyRequest, ThreadMixin OBJECT_ID = "472be9ec-26fe-461b-8282-9c4e4b207ab3" @@ -783,7 +783,6 @@ def test_timestamp_are_incremented_on_delete(self): after = self.storage.resource_timestamp(**self.storage_kw) self.assertTrue(before < after) - @skip_if_travis def test_timestamps_are_unique(self): # pragma: no cover obtained = [] From fc89bb1561d5f1bd4960766745302ea6e2dce48d Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 30 Mar 2020 13:32:11 +0200 Subject: [PATCH 2/4] Leverage objects table index --- kinto/core/storage/postgresql/__init__.py | 2 +- kinto/core/storage/postgresql/migrations/migration_021_022.sql | 2 +- kinto/core/storage/postgresql/schema.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kinto/core/storage/postgresql/__init__.py b/kinto/core/storage/postgresql/__init__.py index 1dd4a5633..275238007 100644 --- a/kinto/core/storage/postgresql/__init__.py +++ b/kinto/core/storage/postgresql/__init__.py @@ -201,7 +201,7 @@ def resource_timestamp(self, resource_name, parent_id): FROM objects WHERE parent_id = :parent_id AND resource_name = :resource_name - ORDER BY last_modified DESC + ORDER BY as_epoch(last_modified) DESC LIMIT 1 ) -- Timestamp of empty resource. diff --git a/kinto/core/storage/postgresql/migrations/migration_021_022.sql b/kinto/core/storage/postgresql/migrations/migration_021_022.sql index b7599f360..e007272f8 100644 --- a/kinto/core/storage/postgresql/migrations/migration_021_022.sql +++ b/kinto/core/storage/postgresql/migrations/migration_021_022.sql @@ -12,7 +12,7 @@ BEGIN FROM objects WHERE parent_id = NEW.parent_id AND resource_name = NEW.resource_name - ORDER BY last_modified DESC + ORDER BY as_epoch(last_modified) DESC LIMIT 1 ) -- Timestamp when resource was empty. diff --git a/kinto/core/storage/postgresql/schema.sql b/kinto/core/storage/postgresql/schema.sql index 65d5c4d2f..d0a9a4413 100644 --- a/kinto/core/storage/postgresql/schema.sql +++ b/kinto/core/storage/postgresql/schema.sql @@ -75,7 +75,7 @@ BEGIN FROM objects WHERE parent_id = NEW.parent_id AND resource_name = NEW.resource_name - ORDER BY last_modified DESC + ORDER BY as_epoch(last_modified) DESC LIMIT 1 ) -- Timestamp when resource was empty. From d36dd3a82bd1b6cc830ad7d667a5e94d85d0c952 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 30 Mar 2020 13:32:44 +0200 Subject: [PATCH 3/4] Replacing a trigger function requires reinstall --- .../storage/postgresql/migrations/migration_021_022.sql | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kinto/core/storage/postgresql/migrations/migration_021_022.sql b/kinto/core/storage/postgresql/migrations/migration_021_022.sql index e007272f8..5eed3f06b 100644 --- a/kinto/core/storage/postgresql/migrations/migration_021_022.sql +++ b/kinto/core/storage/postgresql/migrations/migration_021_022.sql @@ -1,3 +1,5 @@ +DROP TRIGGER IF EXISTS tgr_objects_last_modified ON objects; + CREATE OR REPLACE FUNCTION bump_timestamp() RETURNS trigger AS $$ DECLARE @@ -52,5 +54,9 @@ BEGIN END; $$ LANGUAGE plpgsql; +CREATE TRIGGER tgr_objects_last_modified +BEFORE INSERT OR UPDATE OF data ON objects +FOR EACH ROW EXECUTE PROCEDURE bump_timestamp(); + -- Bump storage schema version. INSERT INTO metadata (name, value) VALUES ('storage_schema_version', '22'); From a83ec55609e5588985eec2771c51ef0b5875c282 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 30 Mar 2020 13:41:16 +0200 Subject: [PATCH 4/4] Tests fail on Travis --- kinto/core/storage/testing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kinto/core/storage/testing.py b/kinto/core/storage/testing.py index 23f7f21b1..76622eb47 100644 --- a/kinto/core/storage/testing.py +++ b/kinto/core/storage/testing.py @@ -5,7 +5,7 @@ from kinto.core import utils from kinto.core.storage import MISSING, Filter, Sort, exceptions, heartbeat -from kinto.core.testing import DummyRequest, ThreadMixin +from kinto.core.testing import DummyRequest, ThreadMixin, skip_if_travis OBJECT_ID = "472be9ec-26fe-461b-8282-9c4e4b207ab3" @@ -783,6 +783,7 @@ def test_timestamp_are_incremented_on_delete(self): after = self.storage.resource_timestamp(**self.storage_kw) self.assertTrue(before < after) + @skip_if_travis def test_timestamps_are_unique(self): # pragma: no cover obtained = []