Skip to content
This repository was archived by the owner on Mar 26, 2024. It is now read-only.

Commit 664b558

Browse files
erikjohnstonFizzadar
erikjohnston
authored and
Fizzadar
committed
Fix race in triggers for read/write locks. (matrix-org#15933)
1 parent cc8aed2 commit 664b558

File tree

5 files changed

+135
-98
lines changed

5 files changed

+135
-98
lines changed

‎changelog.d/15933.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix bug with read/write lock implementation. This is currently unused so has no observable effects.

‎synapse/storage/schema/main/delta/78/04_read_write_locks_triggers.sql.postgres

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -99,54 +99,3 @@ CREATE UNIQUE INDEX worker_read_write_locks_write ON worker_read_write_locks (lo
9999
-- constraints.
100100
ALTER TABLE worker_read_write_locks_mode ADD CONSTRAINT worker_read_write_locks_mode_foreign
101101
FOREIGN KEY (lock_name, lock_key, token) REFERENCES worker_read_write_locks(lock_name, lock_key, token) DEFERRABLE INITIALLY DEFERRED;
102-
103-
104-
-- Add a trigger to UPSERT into `worker_read_write_locks_mode` whenever we try
105-
-- and acquire a lock, i.e. insert into `worker_read_write_locks`,
106-
CREATE OR REPLACE FUNCTION upsert_read_write_lock_parent() RETURNS trigger AS $$
107-
BEGIN
108-
INSERT INTO worker_read_write_locks_mode (lock_name, lock_key, write_lock, token)
109-
VALUES (NEW.lock_name, NEW.lock_key, NEW.write_lock, NEW.token)
110-
ON CONFLICT (lock_name, lock_key)
111-
DO NOTHING;
112-
RETURN NEW;
113-
END
114-
$$
115-
LANGUAGE plpgsql;
116-
117-
CREATE TRIGGER upsert_read_write_lock_parent_trigger BEFORE INSERT ON worker_read_write_locks
118-
FOR EACH ROW
119-
EXECUTE PROCEDURE upsert_read_write_lock_parent();
120-
121-
122-
-- Ensure that we keep `worker_read_write_locks_mode` up to date whenever a lock
123-
-- is released (i.e. a row deleted from `worker_read_write_locks`). Either we
124-
-- update the `worker_read_write_locks_mode.token` to match another instance
125-
-- that has currently acquired the lock, or we delete the row if nobody has
126-
-- currently acquired a lock.
127-
CREATE OR REPLACE FUNCTION delete_read_write_lock_parent() RETURNS trigger AS $$
128-
DECLARE
129-
new_token TEXT;
130-
BEGIN
131-
SELECT token INTO new_token FROM worker_read_write_locks
132-
WHERE
133-
lock_name = OLD.lock_name
134-
AND lock_key = OLD.lock_key;
135-
136-
IF NOT FOUND THEN
137-
DELETE FROM worker_read_write_locks_mode
138-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key;
139-
ELSE
140-
UPDATE worker_read_write_locks_mode
141-
SET token = new_token
142-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key;
143-
END IF;
144-
145-
RETURN NEW;
146-
END
147-
$$
148-
LANGUAGE plpgsql;
149-
150-
CREATE TRIGGER delete_read_write_lock_parent_trigger AFTER DELETE ON worker_read_write_locks
151-
FOR EACH ROW
152-
EXECUTE PROCEDURE delete_read_write_lock_parent();

‎synapse/storage/schema/main/delta/78/04_read_write_locks_triggers.sql.sqlite

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -70,50 +70,3 @@ CREATE TABLE worker_read_write_locks (
7070
CREATE UNIQUE INDEX worker_read_write_locks_key ON worker_read_write_locks (lock_name, lock_key, token);
7171
-- Ensures that only one instance can acquire a lock in write mode at a time.
7272
CREATE UNIQUE INDEX worker_read_write_locks_write ON worker_read_write_locks (lock_name, lock_key) WHERE write_lock;
73-
74-
75-
-- Add a trigger to UPSERT into `worker_read_write_locks_mode` whenever we try
76-
-- and acquire a lock, i.e. insert into `worker_read_write_locks`,
77-
CREATE TRIGGER IF NOT EXISTS upsert_read_write_lock_parent_trigger
78-
BEFORE INSERT ON worker_read_write_locks
79-
FOR EACH ROW
80-
BEGIN
81-
-- First ensure that `worker_read_write_locks_mode` doesn't have stale
82-
-- entries in it, as on SQLite we don't have the foreign key constraint to
83-
-- enforce this.
84-
DELETE FROM worker_read_write_locks_mode
85-
WHERE lock_name = NEW.lock_name AND lock_key = NEW.lock_key
86-
AND NOT EXISTS (
87-
SELECT 1 FROM worker_read_write_locks
88-
WHERE lock_name = NEW.lock_name AND lock_key = NEW.lock_key
89-
);
90-
91-
INSERT INTO worker_read_write_locks_mode (lock_name, lock_key, write_lock, token)
92-
VALUES (NEW.lock_name, NEW.lock_key, NEW.write_lock, NEW.token)
93-
ON CONFLICT (lock_name, lock_key)
94-
DO NOTHING;
95-
END;
96-
97-
-- Ensure that we keep `worker_read_write_locks_mode` up to date whenever a lock
98-
-- is released (i.e. a row deleted from `worker_read_write_locks`). Either we
99-
-- update the `worker_read_write_locks_mode.token` to match another instance
100-
-- that has currently acquired the lock, or we delete the row if nobody has
101-
-- currently acquired a lock.
102-
CREATE TRIGGER IF NOT EXISTS delete_read_write_lock_parent_trigger
103-
AFTER DELETE ON worker_read_write_locks
104-
FOR EACH ROW
105-
BEGIN
106-
DELETE FROM worker_read_write_locks_mode
107-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
108-
AND NOT EXISTS (
109-
SELECT 1 FROM worker_read_write_locks
110-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
111-
);
112-
113-
UPDATE worker_read_write_locks_mode
114-
SET token = (
115-
SELECT token FROM worker_read_write_locks
116-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
117-
)
118-
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key;
119-
END;

‎synapse/storage/schema/main/delta/79/03_read_write_locks_triggers.sql.postgres

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/* Copyright 2023 The Matrix.org Foundation C.I.C
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS,
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
-- Fix up the triggers that were in `78/04_read_write_locks_triggers.sql`
17+
18+
-- Add a trigger to UPSERT into `worker_read_write_locks_mode` whenever we try
19+
-- and acquire a lock, i.e. insert into `worker_read_write_locks`,
20+
CREATE OR REPLACE FUNCTION upsert_read_write_lock_parent() RETURNS trigger AS $$
21+
BEGIN
22+
INSERT INTO worker_read_write_locks_mode (lock_name, lock_key, write_lock, token)
23+
VALUES (NEW.lock_name, NEW.lock_key, NEW.write_lock, NEW.token)
24+
ON CONFLICT (lock_name, lock_key)
25+
DO UPDATE SET write_lock = NEW.write_lock, token = NEW.token;
26+
RETURN NEW;
27+
END
28+
$$
29+
LANGUAGE plpgsql;
30+
31+
DROP TRIGGER IF EXISTS upsert_read_write_lock_parent_trigger ON worker_read_write_locks;
32+
CREATE TRIGGER upsert_read_write_lock_parent_trigger BEFORE INSERT ON worker_read_write_locks
33+
FOR EACH ROW
34+
EXECUTE PROCEDURE upsert_read_write_lock_parent();
35+
36+
37+
-- Ensure that we keep `worker_read_write_locks_mode` up to date whenever a lock
38+
-- is released (i.e. a row deleted from `worker_read_write_locks`). Either we
39+
-- update the `worker_read_write_locks_mode.token` to match another instance
40+
-- that has currently acquired the lock, or we delete the row if nobody has
41+
-- currently acquired a lock.
42+
CREATE OR REPLACE FUNCTION delete_read_write_lock_parent() RETURNS trigger AS $$
43+
DECLARE
44+
new_token TEXT;
45+
BEGIN
46+
SELECT token INTO new_token FROM worker_read_write_locks
47+
WHERE
48+
lock_name = OLD.lock_name
49+
AND lock_key = OLD.lock_key
50+
LIMIT 1 FOR UPDATE;
51+
52+
IF NOT FOUND THEN
53+
DELETE FROM worker_read_write_locks_mode
54+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key AND token = OLD.token;
55+
ELSE
56+
UPDATE worker_read_write_locks_mode
57+
SET token = new_token
58+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key;
59+
END IF;
60+
61+
RETURN NEW;
62+
END
63+
$$
64+
LANGUAGE plpgsql;
65+
66+
DROP TRIGGER IF EXISTS delete_read_write_lock_parent_trigger ON worker_read_write_locks;
67+
CREATE TRIGGER delete_read_write_lock_parent_trigger AFTER DELETE ON worker_read_write_locks
68+
FOR EACH ROW
69+
EXECUTE PROCEDURE delete_read_write_lock_parent();

‎synapse/storage/schema/main/delta/79/03_read_write_locks_triggers.sql.sqlite

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/* Copyright 2023 The Matrix.org Foundation C.I.C
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS,
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
-- Fix up the triggers that were in `78/04_read_write_locks_triggers.sql`
17+
18+
-- Add a trigger to UPSERT into `worker_read_write_locks_mode` whenever we try
19+
-- and acquire a lock, i.e. insert into `worker_read_write_locks`,
20+
DROP TRIGGER IF EXISTS upsert_read_write_lock_parent_trigger;
21+
CREATE TRIGGER IF NOT EXISTS upsert_read_write_lock_parent_trigger
22+
BEFORE INSERT ON worker_read_write_locks
23+
FOR EACH ROW
24+
BEGIN
25+
-- First ensure that `worker_read_write_locks_mode` doesn't have stale
26+
-- entries in it, as on SQLite we don't have the foreign key constraint to
27+
-- enforce this.
28+
DELETE FROM worker_read_write_locks_mode
29+
WHERE lock_name = NEW.lock_name AND lock_key = NEW.lock_key
30+
AND NOT EXISTS (
31+
SELECT 1 FROM worker_read_write_locks
32+
WHERE lock_name = NEW.lock_name AND lock_key = NEW.lock_key
33+
);
34+
35+
INSERT INTO worker_read_write_locks_mode (lock_name, lock_key, write_lock, token)
36+
VALUES (NEW.lock_name, NEW.lock_key, NEW.write_lock, NEW.token)
37+
ON CONFLICT (lock_name, lock_key)
38+
DO UPDATE SET write_lock = NEW.write_lock, token = NEW.token;
39+
END;
40+
41+
-- Ensure that we keep `worker_read_write_locks_mode` up to date whenever a lock
42+
-- is released (i.e. a row deleted from `worker_read_write_locks`). Either we
43+
-- update the `worker_read_write_locks_mode.token` to match another instance
44+
-- that has currently acquired the lock, or we delete the row if nobody has
45+
-- currently acquired a lock.
46+
DROP TRIGGER IF EXISTS delete_read_write_lock_parent_trigger;
47+
CREATE TRIGGER IF NOT EXISTS delete_read_write_lock_parent_trigger
48+
AFTER DELETE ON worker_read_write_locks
49+
FOR EACH ROW
50+
BEGIN
51+
DELETE FROM worker_read_write_locks_mode
52+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
53+
AND token = OLD.token
54+
AND NOT EXISTS (
55+
SELECT 1 FROM worker_read_write_locks
56+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
57+
);
58+
59+
UPDATE worker_read_write_locks_mode
60+
SET token = (
61+
SELECT token FROM worker_read_write_locks
62+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key
63+
)
64+
WHERE lock_name = OLD.lock_name AND lock_key = OLD.lock_key;
65+
END;

0 commit comments

Comments
 (0)