pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] rework locking code in GetMultiXactIdMembers
3+ messages / 2 participants
[nested] [flat]
* [PATCH] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 3+ messages in thread
From: Alvaro Herrera @ 2024-03-03 14:20 UTC (permalink / raw)
Per Coverity
---
src/backend/access/transam/multixact.c | 52 ++++++++++----------------
1 file changed, 20 insertions(+), 32 deletions(-)
inline patch 1: src/backend/access/transam/multixact.c (+20/-32 lines) download inline-1.patch
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 9b81506145..ec446949a9 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1250,14 +1250,12 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
MultiXactOffset offset;
int length;
int truelength;
- int i;
MultiXactId oldestMXact;
MultiXactId nextMXact;
MultiXactId tmpMXact;
MultiXactOffset nextOffset;
MultiXactMember *ptr;
LWLock *lock;
- LWLock *prevlock = NULL;
debug_elog3(DEBUG2, "GetMembers: asked for %u", multi);
@@ -1364,18 +1362,9 @@ retry:
pageno = MultiXactIdToOffsetPage(multi);
entryno = MultiXactIdToOffsetEntry(multi);
- /*
- * If this page falls under a different bank, release the old bank's lock
- * and acquire the lock of the new bank.
- */
+ /* Acquire the bank lock for the page we need. */
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
- if (lock != prevlock)
- {
- if (prevlock != NULL)
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
- }
+ LWLockAcquire(lock, LW_EXCLUSIVE);
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1410,17 +1399,19 @@ retry:
if (pageno != prev_pageno)
{
+ LWLock *newlock;
+
/*
* Since we're going to access a different SLRU page, if this page
* falls under a different bank, release the old bank's lock and
* acquire the lock of the new bank.
*/
- lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
- if (prevlock != lock)
+ newlock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
+ if (newlock != lock)
{
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
+ LWLockRelease(lock);
+ LWLockAcquire(newlock, LW_EXCLUSIVE);
+ lock = newlock;
}
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
}
@@ -1432,8 +1423,8 @@ retry:
if (nextMXOffset == 0)
{
/* Corner case 2: next multixact is still being filled in */
- LWLockRelease(prevlock);
- prevlock = NULL;
+ LWLockRelease(lock);
+ lock = NULL;
CHECK_FOR_INTERRUPTS();
pg_usleep(1000L);
goto retry;
@@ -1442,14 +1433,11 @@ retry:
length = nextMXOffset - offset;
}
- LWLockRelease(prevlock);
- prevlock = NULL;
-
ptr = (MultiXactMember *) palloc(length * sizeof(MultiXactMember));
truelength = 0;
prev_pageno = -1;
- for (i = 0; i < length; i++, offset++)
+ for (int i = 0; i < length; i++, offset++)
{
TransactionId *xactptr;
uint32 *flagsptr;
@@ -1462,18 +1450,19 @@ retry:
if (pageno != prev_pageno)
{
+ LWLock *newlock;
+
/*
* Since we're going to access a different SLRU page, if this page
* falls under a different bank, release the old bank's lock and
* acquire the lock of the new bank.
*/
- lock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
- if (lock != prevlock)
+ newlock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
+ if (newlock != lock)
{
- if (prevlock)
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
+ LWLockRelease(lock);
+ LWLockAcquire(newlock, LW_EXCLUSIVE);
+ lock = newlock;
}
slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
@@ -1499,8 +1488,7 @@ retry:
truelength++;
}
- if (prevlock)
- LWLockRelease(prevlock);
+ LWLockRelease(lock);
/* A multixid with zero members should not happen */
Assert(truelength > 0);
--
2.39.2
* [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 3+ messages in thread
From: Alvaro Herrera @ 2024-03-03 14:20 UTC (permalink / raw)
Per Coverity
---
src/backend/access/transam/multixact.c | 53 +++++++++++---------------
1 file changed, 22 insertions(+), 31 deletions(-)
inline patch 1: src/backend/access/transam/multixact.c (+22/-31 lines) download inline-1.patch
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index cd476b94fa..83b578dced 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1247,14 +1247,12 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
MultiXactOffset offset;
int length;
int truelength;
- int i;
MultiXactId oldestMXact;
MultiXactId nextMXact;
MultiXactId tmpMXact;
MultiXactOffset nextOffset;
MultiXactMember *ptr;
LWLock *lock;
- LWLock *prevlock = NULL;
debug_elog3(DEBUG2, "GetMembers: asked for %u", multi);
@@ -1361,18 +1359,9 @@ retry:
pageno = MultiXactIdToOffsetPage(multi);
entryno = MultiXactIdToOffsetEntry(multi);
- /*
- * If this page falls under a different bank, release the old bank's lock
- * and acquire the lock of the new bank.
- */
+ /* Acquire the bank lock for the page we need. */
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
- if (lock != prevlock)
- {
- if (prevlock != NULL)
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
- }
+ LWLockAcquire(lock, LW_EXCLUSIVE);
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1407,17 +1396,19 @@ retry:
if (pageno != prev_pageno)
{
+ LWLock *newlock;
+
/*
* Since we're going to access a different SLRU page, if this page
* falls under a different bank, release the old bank's lock and
* acquire the lock of the new bank.
*/
- lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
- if (prevlock != lock)
+ newlock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
+ if (newlock != lock)
{
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
+ LWLockRelease(lock);
+ LWLockAcquire(newlock, LW_EXCLUSIVE);
+ lock = newlock;
}
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
}
@@ -1429,8 +1420,7 @@ retry:
if (nextMXOffset == 0)
{
/* Corner case 2: next multixact is still being filled in */
- LWLockRelease(prevlock);
- prevlock = NULL;
+ LWLockRelease(lock);
CHECK_FOR_INTERRUPTS();
pg_usleep(1000L);
goto retry;
@@ -1439,14 +1429,14 @@ retry:
length = nextMXOffset - offset;
}
- LWLockRelease(prevlock);
- prevlock = NULL;
+ LWLockRelease(lock);
+ lock = NULL;
ptr = (MultiXactMember *) palloc(length * sizeof(MultiXactMember));
truelength = 0;
prev_pageno = -1;
- for (i = 0; i < length; i++, offset++)
+ for (int i = 0; i < length; i++, offset++)
{
TransactionId *xactptr;
uint32 *flagsptr;
@@ -1459,18 +1449,20 @@ retry:
if (pageno != prev_pageno)
{
+ LWLock *newlock;
+
/*
* Since we're going to access a different SLRU page, if this page
* falls under a different bank, release the old bank's lock and
* acquire the lock of the new bank.
*/
- lock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
- if (lock != prevlock)
+ newlock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
+ if (newlock != lock)
{
- if (prevlock)
- LWLockRelease(prevlock);
- LWLockAcquire(lock, LW_EXCLUSIVE);
- prevlock = lock;
+ if (lock)
+ LWLockRelease(lock);
+ LWLockAcquire(newlock, LW_EXCLUSIVE);
+ lock = newlock;
}
slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
@@ -1496,8 +1488,7 @@ retry:
truelength++;
}
- if (prevlock)
- LWLockRelease(prevlock);
+ LWLockRelease(lock);
/* A multixid with zero members should not happen */
Assert(truelength > 0);
--
2.39.2
* pgsql: Add some tests for CREATE OR REPLACE VIEW with column additions
@ 2026-03-04 01:11 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 3+ messages in thread
From: Michael Paquier @ 2026-03-04 01:11 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Add some tests for CREATE OR REPLACE VIEW with column additions
When working on an already-defined view with matching attributes, CREATE
OR REPLACE VIEW would internally generate an ALTER TABLE command with a
set of AT_AddColumnToView sub-commands, one for each attribute added.
Such a command is stored in event triggers twice:
- Once as a simple command.
- Once as an ALTER TABLE command, as it has sub-commands.
There was no test coverage to track this command pattern in terms of
event triggers and DDL deparsing:
- For the test module test_ddl_deparse, two command notices are issued.
- For event triggers, a CREATE VIEW command is logged twice, which may
look a bit weird first, but again this maps with the internal behavior
of how the commands are built, and how the event trigger code reacts in
terms of commands gathered.
While on it, this adds a test for CREATE SCHEMA with a CREATE VIEW
command embedded in it, case supported by the grammar but not covered
yet.
This hole in the test coverage has been found while digging into what
would be a similar behavior for sequences if adding attributes to them
with ALTER TABLE variants, after the initial relation creation.
Discussion: https://postgr.es/m/aaFG9bqkEn0RhLJG@paquier.xyz
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/9ef6381829cb92a23ed7730335a669cb16af3580
Modified Files
--------------
src/test/modules/test_ddl_deparse/expected/create_view.out | 6 ++++++
src/test/modules/test_ddl_deparse/sql/create_view.sql | 3 +++
src/test/regress/expected/event_trigger.out | 10 +++++++++-
src/test/regress/sql/event_trigger.sql | 7 ++++++-
4 files changed, 24 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-03-04 01:11 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-03 14:20 [PATCH] rework locking code in GetMultiXactIdMembers Alvaro Herrera <alvherre@alvh.no-ip.org>
2024-03-03 14:20 [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers Alvaro Herrera <alvherre@alvh.no-ip.org>
2026-03-04 01:11 pgsql: Add some tests for CREATE OR REPLACE VIEW with column additions Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox