public inbox for [email protected]
help / color / mirror / Atom feedRe: Fix search_path for all maintenance commands
10+ messages / 4 participants
[nested] [flat]
* Re: Fix search_path for all maintenance commands
@ 2024-01-18 03:54 Shubham Khanna <[email protected]>
2024-01-18 05:11 ` Re: Fix search_path for all maintenance commands Jeff Davis <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Shubham Khanna @ 2024-01-18 03:54 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; David G. Johnston <[email protected]>; Gurjeet Singh <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Tom Lane <[email protected]>
On Thu, Jan 18, 2024 at 9:19 AM Jeff Davis <[email protected]> wrote:
>
> On Mon, 2023-07-17 at 12:16 -0700, Jeff Davis wrote:
> > Based on feedback, I plan to commit soon.
>
> Attached is a new version.
>
> Changes:
>
> * Also switch the search_path during CREATE MATERIALIZED VIEW, so that
> it's consistent with REFRESH. As a part of this change, I slightly
> reordered things in ExecCreateTableAs() so that the skipData path
> returns early without entering the SECURITY_RESTRICTED_OPERATION. I
> don't think that's a problem because (a) that is one place where
> SECURITY_RESTRICTED_OPERATION is not used for security, but rather for
> consistency; and (b) that path doesn't go through rewriter, planner, or
> executor anyway so I don't see why it would matter.
>
> * Use GUC_ACTION_SAVE rather than GUC_ACTION_SET. That was a problem
> with the previous patch for index functions executed in parallel
> workers, which can happen calling SQL functions from pg_amcheck.
>
> * I used a wrapper function RestrictSearchPath() rather than calling
> set_config_option() directly. That provides a nice place in case we
> need to add a compatibility GUC to disable it.
>
> Question:
>
> Why do we switch to the table owner and use
> SECURITY_RESTRICTED_OPERATION in DefineIndex(), when we will switch in
> index_build (etc.) anyway? Similarly, why do we switch in vacuum_rel(),
> when it doesn't matter for lazy vacuum and we will switch in
> cluster_rel() and do_analyze_rel() anyway?
I tried to apply the patch but it is failing at the Head. It is giving
the following error:
Hunk #7 succeeded at 3772 (offset -12 lines).
patching file src/backend/commands/matview.c
patching file src/backend/commands/vacuum.c
Hunk #2 succeeded at 2169 (offset -19 lines).
patching file src/backend/utils/init/usercontext.c
patching file src/bin/scripts/t/100_vacuumdb.pl
Hunk #1 FAILED at 109.
1 out of 1 hunk FAILED -- saving rejects to file
src/bin/scripts/t/100_vacuumdb.pl.rej
patching file src/include/utils/usercontext.h
patching file src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
patching file src/test/regress/expected/matview.out
patching file src/test/regress/expected/privileges.out
patching file src/test/regress/expected/vacuum.out
patching file src/test/regress/sql/matview.sql
patching file src/test/regress/sql/privileges.sql
patching file src/test/regress/sql/vacuum.sql
Please send the Re-base version of the patch.
Thanks and Regards,
Shubham Khanna.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix search_path for all maintenance commands
2024-01-18 03:54 Re: Fix search_path for all maintenance commands Shubham Khanna <[email protected]>
@ 2024-01-18 05:11 ` Jeff Davis <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Davis @ 2024-01-18 05:11 UTC (permalink / raw)
To: Shubham Khanna <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; David G. Johnston <[email protected]>; Gurjeet Singh <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Tom Lane <[email protected]>
On Thu, 2024-01-18 at 09:24 +0530, Shubham Khanna wrote:
> I tried to apply the patch but it is failing at the Head. It is
> giving
> the following error:
I am withdrawing this patch from the CF until it's more clear that this
is what we really want to do.
Thank you for looking into it.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--rb5vb6debo3bx6lt--
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--rb5vb6debo3bx6lt--
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--rb5vb6debo3bx6lt--
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--gflnge3qfnp343ni
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v2-0002-Rework-redundant-loop-in-subtrans.c.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--gflnge3qfnp343ni
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v2-0002-Rework-redundant-loop-in-subtrans.c.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--gflnge3qfnp343ni
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v2-0002-Rework-redundant-loop-in-subtrans.c.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH] rework locking code in GetMultiXactIdMembers
@ 2024-03-03 14:20 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 10+ 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(-)
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
--rb5vb6debo3bx6lt--
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Windows 2016 server crashed after changes in Postgres 15.8 pgAdmin
@ 2024-11-20 13:41 Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gustafsson @ 2024-11-20 13:41 UTC (permalink / raw)
To: Sanjay Khatri <[email protected]>; +Cc: Tomas Vondra <[email protected]>; [email protected]
> On 20 Nov 2024, at 14:34, Sanjay Khatri <[email protected]> wrote:
>
> These are the errors from the logs of iDrac M630.
> I uninstalled the Postgres 15.8 , once I got the connection. But sadly the server disconnected again and crashed, even after uninstalling.
> No I am not able to boot it.
I'm going to go out on a limb and say that this isn't a postgres bug, and
highly unlikely to be caused by one. This smells like broken hardware and/or
corrupted BIOS/firmware, you should contact your hardware vendor for support.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-11-20 13:41 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-18 03:54 Re: Fix search_path for all maintenance commands Shubham Khanna <[email protected]>
2024-01-18 05:11 ` Jeff Davis <[email protected]>
2024-03-03 14:20 [PATCH] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH v2 1/3] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-03-03 14:20 [PATCH] rework locking code in GetMultiXactIdMembers Alvaro Herrera <[email protected]>
2024-11-20 13:41 Re: Windows 2016 server crashed after changes in Postgres 15.8 pgAdmin Daniel Gustafsson <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox