On Wed, Feb 8, 2023 at 10:44 AM Thomas Munro wrote:
> On Wed, Feb 8, 2023 at 5:25 AM Andrey Borodin wrote:
> > On Tue, Feb 7, 2023 at 4:01 AM Rinat Shigapov
> > wrote:
> > > Thomas, thank you for the details!
> > >
> > > Have you kept the branch that you used to generate the patch? Which
> > > commit should the patch apply to?
> >
> > You can try something like
> > git checkout 'master@{2018-05-13 13:37:00}'
> > to get a commit by date from rev-parse.
>
> I don't have time to work on this currently but if Rinat or others
> want to look into it... maybe I should rebase that experiment on top
> of current master. Here's the branch:
>
> https://github.com/macdice/postgres/tree/ssi-index-locking-refinements
Erm, I guess I should also post the rebased patches here, for the
mailing list archives.
From 299e2dfb54ffabe45d73e3296856132169a7419b Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Thu, 10 Jun 2021 23:35:16 +1200
Subject: [PATCH v5 1/2] Use tuple-level SIREAD locks for index-only scans.
When index-only scans manage to avoid fetching heap tuples,
previously we'd predicate-lock the whole heap page (since commit
cdf91edb). Commits c01262a8 and 6f38d4da made it possible to lock the
tuple instead with only a TID, to match the behavior of regular index
scans and avoid some false conflicts.
Discussion: https://postgr.es/m/CAEepm%3D2GK3FVdnt5V3d%2Bh9njWipCv_fNL%3DwjxyUhzsF%3D0PcbNg%40mail.gmail.com
---
src/backend/executor/nodeIndexonlyscan.c | 12 --
.../isolation/expected/index-only-scan-2.out | 19 +
.../isolation/expected/index-only-scan-3.out | 20 ++
src/test/isolation/isolation_schedule | 2 +
.../isolation/specs/index-only-scan-2.spec| 39 +++
.../isolation/specs/index-only-scan-3.spec| 33
6 files changed, 121 insertions(+), 4 deletions(-)
create mode 100644 src/test/isolation/expected/index-only-scan-2.out
create mode 100644 src/test/isolation/expected/index-only-scan-3.out
create mode 100644 src/test/isolation/specs/index-only-scan-2.spec
create mode 100644 src/test/isolation/specs/index-only-scan-3.spec
diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
index 0b43a9b969..237f17b07a 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -239,12 +239,16 @@ IndexOnlyNext(IndexOnlyScanState *node)
/*
* If we didn't access the heap, then we'll need to take a predicate
- * lock explicitly, as if we had. For now we do that at page level.
+ * lock explicitly, as if we had. We don't have the inserter's xid,
+ * but that's only used by PredicateLockTID to check if it matches the
+ * caller's top level xid, and it can't possibly have been inserted by
+ * us or the page wouldn't be all visible.
*/
if (!tuple_from_heap)
- PredicateLockPage(scandesc->heapRelation,
- ItemPointerGetBlockNumber(tid),
- estate->es_snapshot);
+ PredicateLockTID(scandesc->heapRelation,
+ tid,
+ estate->es_snapshot,
+ InvalidTransactionId);
return slot;
}
diff --git a/src/test/isolation/expected/index-only-scan-2.out b/src/test/isolation/expected/index-only-scan-2.out
new file mode 100644
index 00..c541eaeb43
--- /dev/null
+++ b/src/test/isolation/expected/index-only-scan-2.out
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: r1 r2 w1 w2 c1 c2
+step r1: SELECT id1 FROM account WHERE id1 = 1;
+id1
+---
+ 1
+(1 row)
+
+step r2: SELECT id2 FROM account WHERE id2 = 2;
+id2
+---
+ 2
+(1 row)
+
+step w1: UPDATE account SET balance = 200 WHERE id1 = 1;
+step w2: UPDATE account SET balance = 200 WHERE id2 = 2;
+step c1: COMMIT;
+step c2: COMMIT;
diff --git a/src/test/isolation/expected/index-only-scan-3.out b/src/test/isolation/expected/index-only-scan-3.out
new file mode 100644
index 00..0a7c5955d2
--- /dev/null
+++ b/src/test/isolation/expected/index-only-scan-3.out
@@ -0,0 +1,20 @@
+Parsed test spec with 2 sessions
+
+starting permutation: r1 r2 w1 w2 c1 c2
+step r1: SELECT id1 FROM account WHERE id1 = 2;
+id1
+---
+ 2
+(1 row)
+
+step r2: SELECT id2 FROM account WHERE id2 = 1;
+id2
+---
+ 1
+(1 row)
+
+step w1: UPDATE account SET balance = 200 WHERE id1 = 1;
+step w2: UPDATE account SET balance = 200 WHERE id2 = 2;
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among transactions
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index c11dc9a420..e1574ae9f7 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -17,6 +17,8 @@ test: partial-index
test: two-ids
test: multiple-row-versions
test: index-only-scan
+test: index-only-scan-2
+test: index-only-scan-3
test: predicate-lock-hot-tuple
test: update-conflict-out
test: deadlock-simple
diff --git