Re: SI-read predicate locks on materialized views
On Thu, 1 Dec 2022 15:48:21 +0900 Michael Paquier wrote: > On Tue, Oct 18, 2022 at 05:29:58PM +0900, Yugo NAGATA wrote: > > Thank you for your review. I agree that an isolation test is required. > > The attached patch contains the test using the scenario as explained in > > the previous post. > > Cool, thanks. Sorry for my late reply here. I have put my head on > that for a few hours and could not see why we should not allow that. > So committed the change after a few tweaks to the tests with the use > of custom permutations, mainly. Thank! > While looking at all that, I have looked at the past threads like [1], > just to note that this has never been really mentioned. > > [1]: > https://www.postgresql.org/message-id/1371225929.28496.yahoomail...@web162905.mail.bf1.yahoo.com > -- > Michael -- Yugo NAGATA
Re: SI-read predicate locks on materialized views
On Tue, Oct 18, 2022 at 05:29:58PM +0900, Yugo NAGATA wrote: > Thank you for your review. I agree that an isolation test is required. > The attached patch contains the test using the scenario as explained in > the previous post. Cool, thanks. Sorry for my late reply here. I have put my head on that for a few hours and could not see why we should not allow that. So committed the change after a few tweaks to the tests with the use of custom permutations, mainly. While looking at all that, I have looked at the past threads like [1], just to note that this has never been really mentioned. [1]: https://www.postgresql.org/message-id/1371225929.28496.yahoomail...@web162905.mail.bf1.yahoo.com -- Michael signature.asc Description: PGP signature
Re: SI-read predicate locks on materialized views
Hello Micheal-san,
On Thu, 13 Oct 2022 17:02:06 +0900
Michael Paquier wrote:
> On Fri, Sep 30, 2022 at 10:12:13AM +0900, Yugo NAGATA wrote:
> > Thank you for comment. Do you think it can be marked as Ready for Commiter?
>
> Matviews have been discarded from needing predicate locks since
> 3bf3ab8 and their introduction, where there was no concurrent flavor
> of refresh yet. Shouldn't this patch have at least an isolation test
> to show the difference in terms of read-write conflicts with some
> serializable transactions and REFRESH CONCURRENTLY?
Thank you for your review. I agree that an isolation test is required.
The attached patch contains the test using the scenario as explained in
the previous post.
Regards,
Yugo Nagata
--
Yugo NAGATA
>From b60a53a945283de4b068e1bc7aaafec26dd8f4da Mon Sep 17 00:00:00 2001
From: Yugo Nagata
Date: Tue, 18 Oct 2022 17:15:33 +0900
Subject: [PATCH] SI-read predicate locking on materialized views
Matviews have been discarded from needing predicate locks since
3bf3ab8 and their introduction, where there was no concurrent flavor
of refresh yet.
---
src/backend/storage/lmgr/predicate.c | 5 +-
.../isolation/expected/matview-write-skew.out | 77 +++
src/test/isolation/isolation_schedule | 1 +
.../isolation/specs/matview-write-skew.spec | 43 +++
4 files changed, 123 insertions(+), 3 deletions(-)
create mode 100644 src/test/isolation/expected/matview-write-skew.out
create mode 100644 src/test/isolation/specs/matview-write-skew.spec
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index e8120174d6..df1c0d72e9 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -490,14 +490,13 @@ static void ReleasePredicateLocksLocal(void);
/*
* Does this relation participate in predicate locking? Temporary and system
- * relations are exempt, as are materialized views.
+ * relations are exempt.
*/
static inline bool
PredicateLockingNeededForRelation(Relation relation)
{
return !(relation->rd_id < FirstUnpinnedObjectId ||
- RelationUsesLocalBuffers(relation) ||
- relation->rd_rel->relkind == RELKIND_MATVIEW);
+ RelationUsesLocalBuffers(relation));
}
/*
diff --git a/src/test/isolation/expected/matview-write-skew.out b/src/test/isolation/expected/matview-write-skew.out
new file mode 100644
index 00..c1f8b3f5ea
--- /dev/null
+++ b/src/test/isolation/expected/matview-write-skew.out
@@ -0,0 +1,77 @@
+Parsed test spec with 2 sessions
+
+starting permutation: rxwy1 c1 rywx2 c2
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step c1: COMMIT;
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step c2: COMMIT;
+
+starting permutation: rxwy1 rywx2 c1 c2
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: rxwy1 rywx2 c2 c1
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step c2: COMMIT;
+step c1: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: rywx2 rxwy1 c1 c2
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: rywx2 rxwy1 c2 c1
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step c2: COMMIT;
+step c1: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
+
+starting permutation: rywx2 c2 rxwy1 c1
+step rywx2:
+ DO $$DECLARE today date;
+ BEGIN
+SELECT INTO today max(date) + 1 FROM order_summary;
+INSERT INTO orders VALUES (today, 'banana', 10);
+ END$$;
+
+step c2: COMMIT;
+step rxwy1: REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
+step c1: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 5413a59a80..c11dc9a420 100644
--- a/src/test/isolation/isolation_schedule
+++
Re: SI-read predicate locks on materialized views
On Fri, Sep 30, 2022 at 10:12:13AM +0900, Yugo NAGATA wrote: > Thank you for comment. Do you think it can be marked as Ready for Commiter? Matviews have been discarded from needing predicate locks since 3bf3ab8 and their introduction, where there was no concurrent flavor of refresh yet. Shouldn't this patch have at least an isolation test to show the difference in terms of read-write conflicts with some serializable transactions and REFRESH CONCURRENTLY? -- Michael signature.asc Description: PGP signature
Re: SI-read predicate locks on materialized views
On Fri, 9 Sep 2022 16:27:45 +0530 Dilip Kumar wrote: > On Tue, Jul 26, 2022 at 3:31 PM Richard Guo wrote: > > > > > > On Tue, Jul 26, 2022 at 3:44 PM Yugo NAGATA wrote: > >> > >> If such two transactions run concurrently, a write skew anomaly occurs, > >> and the result of order_summary refreshed in T1 will not contain the > >> record inserted in T2. > > Yes we do have write skew anomaly. I think the patch looks fine to me. Thank you for comment. Do you think it can be marked as Ready for Commiter? Regards, Yugo Nagata > > -- > Regards, > Dilip Kumar > EnterpriseDB: http://www.enterprisedb.com -- Yugo NAGATA
Re: SI-read predicate locks on materialized views
On Tue, Jul 26, 2022 at 3:31 PM Richard Guo wrote: > > > On Tue, Jul 26, 2022 at 3:44 PM Yugo NAGATA wrote: >> >> If such two transactions run concurrently, a write skew anomaly occurs, >> and the result of order_summary refreshed in T1 will not contain the >> record inserted in T2. Yes we do have write skew anomaly. I think the patch looks fine to me. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
Re: SI-read predicate locks on materialized views
On Tue, Jul 26, 2022 at 3:44 PM Yugo NAGATA wrote: > If such two transactions run concurrently, a write skew anomaly occurs, > and the result of order_summary refreshed in T1 will not contain the > record inserted in T2. Indeed we have write skew anomaly here between the two transactions. > On the other hand, if the materialized view participates in predicate > locking and the transaction isolation level is SELIALIZABLE, this > anomaly can be avoided; one of the transaction will be aborted and > suggested to be retried. The idea works for me. Thanks Richard
SI-read predicate locks on materialized views
Hi,
I propose to acquire SI-read predicate locks on materialized views
as the attached patch.
Currently, materialized views do not participate in predicate locking,
but I think this causes a serialization anomaly when `REFRESH
MATERIALIZED VIEW CONCURRENTLY` is used.
For example, supporse that there is a table "orders" which contains
order information and a materialized view "order_summary" which contains
summary of the order information.
CREATE TABLE orders (date date, item text, num int);
CREATE MATERIALIZED VIEW order_summary AS
SELECT date, item, sum(num) FROM orders GROUP BY date, item;
"order_summary" is refreshed once per day in the following transaction.
T1:
REFRESH MATERIALIZED VIEW CONCURRENTLY order_summary;
"orders" has a date column, and when a new item is inserted, the date
value is determined as the next day of the last date recorded in
"order_summary" as in the following transaction.
T2:
SELECT max(date) + 1 INTO today FROM order_summary;
INSERT INTO orders(date, item, num) VALUES (today, 'apple', 1);
If such two transactions run concurrently, a write skew anomaly occurs,
and the result of order_summary refreshed in T1 will not contain the
record inserted in T2.
On the other hand, if the materialized view participates in predicate
locking and the transaction isolation level is SELIALIZABLE, this
anomaly can be avoided; one of the transaction will be aborted and
suggested to be retried.
The problem doesn't occur when we use REFRESH MATERIALIZED VIEW
(not CONCURRENTLY) because it acquires the strongest lock and
any concurrent transactions are prevent from reading the materialized view.
I think this is the reason why materialized views didn't have to
participate in predicate locking. However, this is no longer the case
because now we support REFRESH ... CONCURRENTLY which refreshes the
materialized view using DELETE and INSERT and also allow to read it
from concurrent transactions. I think we can regard them as same as
DELETE, INSERT, and SELECT on regular tables and acquire predicate
locks on materialized views as well.
What do you think about it?
Regards,
Yugo Nagata
--
Yugo NAGATA
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 5136da6ea3..6d414bfcc9 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -489,14 +489,13 @@ static void ReleasePredicateLocksLocal(void);
/*
* Does this relation participate in predicate locking? Temporary and system
- * relations are exempt, as are materialized views.
+ * relations are exempt.
*/
static inline bool
PredicateLockingNeededForRelation(Relation relation)
{
return !(relation->rd_id < FirstUnpinnedObjectId ||
- RelationUsesLocalBuffers(relation) ||
- relation->rd_rel->relkind == RELKIND_MATVIEW);
+ RelationUsesLocalBuffers(relation));
}
/*
