public inbox for [email protected]
help / color / mirror / Atom feedRe: Partitioned tables and [un]loggedness
15+ messages / 8 participants
[nested] [flat]
* Re: Partitioned tables and [un]loggedness
@ 2025-06-04 09:55 Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Shinya Kato @ 2025-06-04 09:55 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Wed, Jun 4, 2025 at 6:42 PM Michael Paquier <[email protected]> wrote:
>
> On Thu, Sep 19, 2024 at 01:08:33PM +0900, Michael Paquier wrote:
> > I have applied 0001 for now to add ATT_PARTITIONED_TABLE. Attached is
> > the remaining piece.
>
> And the second piece is now applied as of e2bab2d79204.
> --
> Michael
Hi,
Should we consider preventing tab completion for PARTITION BY
immediately after CREATE TABLE name (...)? Or is it fine to leave it
as is, given that it's syntactically correct?
--
Best regards,
Shinya Kato
NTT OSS Center
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
@ 2025-06-04 10:05 ` Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Shinya Kato @ 2025-06-04 10:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Wed, Jun 4, 2025 at 6:55 PM Shinya Kato <[email protected]> wrote:
>
> On Wed, Jun 4, 2025 at 6:42 PM Michael Paquier <[email protected]> wrote:
> >
> > On Thu, Sep 19, 2024 at 01:08:33PM +0900, Michael Paquier wrote:
> > > I have applied 0001 for now to add ATT_PARTITIONED_TABLE. Attached is
> > > the remaining piece.
> >
> > And the second piece is now applied as of e2bab2d79204.
> > --
> > Michael
>
> Hi,
>
> Should we consider preventing tab completion for PARTITION BY
> immediately after CREATE TABLE name (...)? Or is it fine to leave it
> as is, given that it's syntactically correct?
Sorry.
CREATE TABLE name (...) -> CREATE UNLOGGED TABLE name (...)
--
Best regards,
Shinya Kato
NTT OSS Center
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
@ 2025-06-04 16:15 ` Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Nathan Bossart @ 2025-06-04 16:15 UTC (permalink / raw)
To: Shinya Kato <[email protected]>; +Cc: Michael Paquier <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Wed, Jun 04, 2025 at 07:05:20PM +0900, Shinya Kato wrote:
> On Wed, Jun 4, 2025 at 6:55 PM Shinya Kato <[email protected]> wrote:
>> Should we consider preventing tab completion for PARTITION BY
>> immediately after CREATE TABLE name (...)? Or is it fine to leave it
>> as is, given that it's syntactically correct?
>
> Sorry.
> CREATE TABLE name (...) -> CREATE UNLOGGED TABLE name (...)
I see no benefit in recommending things that are guaranteed to error. In
commit 5c1ce1b, we removed tab completion for CREATE UNLOGGED MATERIALIZED
VIEW even though it is supported by the grammar. The partitioned table
case sounds like roughly the same situation.
--
nathan
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
@ 2025-06-05 00:23 ` Michael Paquier <[email protected]>
2025-06-05 01:04 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2025-06-05 00:23 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Shinya Kato <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Wed, Jun 04, 2025 at 11:15:29AM -0500, Nathan Bossart wrote:
> I see no benefit in recommending things that are guaranteed to error. In
> commit 5c1ce1b, we removed tab completion for CREATE UNLOGGED MATERIALIZED
> VIEW even though it is supported by the grammar. The partitioned table
> case sounds like roughly the same situation.
Agreed to not suggest the PARTITION BY clause in the tab completion as
it is not supported by the backend for unlogged tables.
tab-complete.in.c has some handling for CREATE UNLOGGED TABLE around
line 3667, so we could just have an extra case for it, like in the
attached patch. A split already exists for temporary tables to handle
the ON COMMIT clause after the attribute list.
Thoughts?
--
Michael
Attachments:
[text/x-diff] psql-tab-unlogged.patch (1.0K, ../../[email protected]/2-psql-tab-unlogged.patch)
download | inline diff:
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index ec65ab79fecb..620830feb9d7 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -3664,9 +3664,10 @@ match_previous_words(int pattern_id,
TailMatches("CREATE", "TEMP|TEMPORARY|UNLOGGED", "TABLE", MatchAny, "(*)", "AS"))
COMPLETE_WITH("EXECUTE", "SELECT", "TABLE", "VALUES", "WITH");
/* Complete CREATE TABLE name (...) with supported options */
- else if (TailMatches("CREATE", "TABLE", MatchAny, "(*)") ||
- TailMatches("CREATE", "UNLOGGED", "TABLE", MatchAny, "(*)"))
+ else if (TailMatches("CREATE", "TABLE", MatchAny, "(*)"))
COMPLETE_WITH("AS", "INHERITS (", "PARTITION BY", "USING", "TABLESPACE", "WITH (");
+ else if (TailMatches("CREATE", "UNLOGGED", "TABLE", MatchAny, "(*)"))
+ COMPLETE_WITH("AS", "INHERITS (", "USING", "TABLESPACE", "WITH (");
else if (TailMatches("CREATE", "TEMP|TEMPORARY", "TABLE", MatchAny, "(*)"))
COMPLETE_WITH("AS", "INHERITS (", "ON COMMIT", "PARTITION BY", "USING",
"TABLESPACE", "WITH (");
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
@ 2025-06-05 01:04 ` Shinya Kato <[email protected]>
2025-06-05 04:57 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Shinya Kato @ 2025-06-05 01:04 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Thu, Jun 5, 2025 at 9:23 AM Michael Paquier <[email protected]> wrote:
> Agreed to not suggest the PARTITION BY clause in the tab completion as
> it is not supported by the backend for unlogged tables.
> tab-complete.in.c has some handling for CREATE UNLOGGED TABLE around
> line 3667, so we could just have an extra case for it, like in the
> attached patch. A split already exists for temporary tables to handle
> the ON COMMIT clause after the attribute list.
>
> Thoughts?
Thank you. It looks good to me.
--
Best regards,
Shinya Kato
NTT OSS Center
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 01:04 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
@ 2025-06-05 04:57 ` Michael Paquier <[email protected]>
2025-06-05 20:04 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2025-06-05 04:57 UTC (permalink / raw)
To: Shinya Kato <[email protected]>; +Cc: Nathan Bossart <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>
On Thu, Jun 05, 2025 at 10:04:23AM +0900, Shinya Kato wrote:
> Thank you. It looks good to me.
How does the RMT feel about this change? Nathan, would you be OK with
that? It's not a big problem, as well, if the code is kept as-is, but
as it's a simple change..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 01:04 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-05 04:57 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
@ 2025-06-05 20:04 ` Nathan Bossart <[email protected]>
2025-06-06 01:55 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Nathan Bossart @ 2025-06-05 20:04 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Shinya Kato <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>; [email protected]
On Thu, Jun 05, 2025 at 01:57:30PM +0900, Michael Paquier wrote:
> How does the RMT feel about this change? Nathan, would you be OK with
> that? It's not a big problem, as well, if the code is kept as-is, but
> as it's a simple change..
IMHO a case can be reasonably made that this is an oversight in the related
commit. I've added the rest of the RMT here in case they see it
differently.
The patch LGTM, too.
--
nathan
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 01:04 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-05 04:57 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 20:04 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
@ 2025-06-06 01:55 ` Michael Paquier <[email protected]>
2025-06-11 00:28 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2025-06-06 01:55 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Shinya Kato <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>; [email protected]
On Thu, Jun 05, 2025 at 03:04:45PM -0500, Nathan Bossart wrote:
> IMHO a case can be reasonably made that this is an oversight in the related
> commit. I've added the rest of the RMT here in case they see it
> differently.
>
> The patch LGTM, too.
Okay, thanks. Let's wait for a couple of days in case there are more
opinions and/or comments. I propose to apply the patch around the
beginning of next week if there are no objections.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Partitioned tables and [un]loggedness
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 16:15 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 01:04 ` Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-05 04:57 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
2025-06-05 20:04 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[email protected]>
2025-06-06 01:55 ` Re: Partitioned tables and [un]loggedness Michael Paquier <[email protected]>
@ 2025-06-11 00:28 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Michael Paquier @ 2025-06-11 00:28 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Shinya Kato <[email protected]>; David G. Johnston <[email protected]>; Postgres hackers <[email protected]>; [email protected]
On Fri, Jun 06, 2025 at 10:55:32AM +0900, Michael Paquier wrote:
> Okay, thanks. Let's wait for a couple of days in case there are more
> opinions and/or comments. I propose to apply the patch around the
> beginning of next week if there are no objections.
It took a bit longer than wanted, but applied now.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Can we remove support for standard_conforming_strings = off yet?
@ 2026-01-21 10:32 Julien Rouhaud <[email protected]>
2026-01-21 20:09 ` Re: Can we remove support for standard_conforming_strings = off yet? Tom Lane <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Julien Rouhaud @ 2026-01-21 10:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: John Naylor <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]
Hi,
On Tue, Jan 20, 2026 at 01:31:53PM -0500, Tom Lane wrote:
> So, nobody's actually spoken against this change. I think we should
> go forward with it, for the reasons I gave at the top of the thread
> and because "if not now, when?". I have little doubt that if there
> is anyone still running with standard_conforming_strings = off, they
> aren't going to change until forced to. So unless we want to live
> with the potential security hazard forever, we're going to have to
> make a breaking change sometime.
+1 from me. All of that is sensible, plus from an extension point of view it's
easy to write code that is broken with standard_conforming_strings = off so
it's one less thing to worry about.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Can we remove support for standard_conforming_strings = off yet?
2026-01-21 10:32 Re: Can we remove support for standard_conforming_strings = off yet? Julien Rouhaud <[email protected]>
@ 2026-01-21 20:09 ` Tom Lane <[email protected]>
2026-01-21 20:25 ` Re: Can we remove support for standard_conforming_strings = off yet? Bruce Momjian <[email protected]>
2026-01-30 10:10 ` Re: Can we remove support for standard_conforming_strings = off yet? jian he <[email protected]>
0 siblings, 2 replies; 15+ messages in thread
From: Tom Lane @ 2026-01-21 20:09 UTC (permalink / raw)
To: Julien Rouhaud <[email protected]>; +Cc: John Naylor <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]
Julien Rouhaud <[email protected]> writes:
> On Tue, Jan 20, 2026 at 01:31:53PM -0500, Tom Lane wrote:
>> So, nobody's actually spoken against this change. I think we should
>> go forward with it, for the reasons I gave at the top of the thread
>> and because "if not now, when?". I have little doubt that if there
>> is anyone still running with standard_conforming_strings = off, they
>> aren't going to change until forced to. So unless we want to live
>> with the potential security hazard forever, we're going to have to
>> make a breaking change sometime.
> +1 from me. All of that is sensible, plus from an extension point of view it's
> easy to write code that is broken with standard_conforming_strings = off so
> it's one less thing to worry about.
Pushed.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Can we remove support for standard_conforming_strings = off yet?
2026-01-21 10:32 Re: Can we remove support for standard_conforming_strings = off yet? Julien Rouhaud <[email protected]>
2026-01-21 20:09 ` Re: Can we remove support for standard_conforming_strings = off yet? Tom Lane <[email protected]>
@ 2026-01-21 20:25 ` Bruce Momjian <[email protected]>
1 sibling, 0 replies; 15+ messages in thread
From: Bruce Momjian @ 2026-01-21 20:25 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; John Naylor <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]
On Wed, Jan 21, 2026 at 03:09:30PM -0500, Tom Lane wrote:
> Julien Rouhaud <[email protected]> writes:
> > On Tue, Jan 20, 2026 at 01:31:53PM -0500, Tom Lane wrote:
> >> So, nobody's actually spoken against this change. I think we should
> >> go forward with it, for the reasons I gave at the top of the thread
> >> and because "if not now, when?". I have little doubt that if there
> >> is anyone still running with standard_conforming_strings = off, they
> >> aren't going to change until forced to. So unless we want to live
> >> with the potential security hazard forever, we're going to have to
> >> make a breaking change sometime.
>
> > +1 from me. All of that is sensible, plus from an extension point of view it's
> > easy to write code that is broken with standard_conforming_strings = off so
> > it's one less thing to worry about.
>
> Pushed.
Great, thanks.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Do not let urgent matters crowd out time for investment in the future.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Can we remove support for standard_conforming_strings = off yet?
2026-01-21 10:32 Re: Can we remove support for standard_conforming_strings = off yet? Julien Rouhaud <[email protected]>
2026-01-21 20:09 ` Re: Can we remove support for standard_conforming_strings = off yet? Tom Lane <[email protected]>
@ 2026-01-30 10:10 ` jian he <[email protected]>
1 sibling, 0 replies; 15+ messages in thread
From: jian he @ 2026-01-30 10:10 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; John Naylor <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]
hi.
_allocAH is called by OpenArchive and CreateArchive. in _allocAH, we can set
AH->public.std_strings = true;
that would be more future-proof, otherwise after OpenArchive,
executing the query ``SET standard_conforming_strings = off;``
would result in an error.
--
jian
https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v1 8/8] convert FastPathStrongRelationLocks to atomics
@ 2026-07-09 20:38 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2026-07-09 20:38 UTC (permalink / raw)
---
src/backend/storage/lmgr/lock.c | 57 ++++++++++-----------------------
1 file changed, 17 insertions(+), 40 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 5ee80c7632e..035932197a4 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -40,11 +40,11 @@
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
+#include "port/atomics.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/shmem.h"
-#include "storage/spin.h"
#include "storage/standby.h"
#include "storage/subsystems.h"
#include "utils/memutils.h"
@@ -306,13 +306,7 @@ static PROCLOCK *FastPathGetRelationLockEntry(LOCALLOCK *locallock);
#define FastPathStrongLockHashPartition(hashcode) \
((hashcode) % FAST_PATH_STRONG_LOCK_HASH_PARTITIONS)
-typedef struct
-{
- slock_t mutex;
- uint32 count[FAST_PATH_STRONG_LOCK_HASH_PARTITIONS];
-} FastPathStrongRelationLockData;
-
-static FastPathStrongRelationLockData *FastPathStrongRelationLocks;
+static pg_atomic_uint32 *FastPathStrongRelationLocks;
static void LockManagerShmemRequest(void *arg);
static void LockManagerShmemInit(void *arg);
@@ -484,7 +478,8 @@ LockManagerShmemRequest(void *arg)
);
ShmemRequestStruct(.name = "Fast Path Strong Relation Lock Data",
- .size = sizeof(FastPathStrongRelationLockData),
+ .size = mul_size(sizeof(pg_atomic_uint32),
+ FAST_PATH_STRONG_LOCK_HASH_PARTITIONS),
.ptr = (void **) (void *) &FastPathStrongRelationLocks,
);
}
@@ -492,7 +487,8 @@ LockManagerShmemRequest(void *arg)
static void
LockManagerShmemInit(void *arg)
{
- SpinLockInit(&FastPathStrongRelationLocks->mutex);
+ for (int i = 0; i < FAST_PATH_STRONG_LOCK_HASH_PARTITIONS; i++)
+ pg_atomic_init_u32(&FastPathStrongRelationLocks[i], 0);
}
/*
@@ -992,11 +988,11 @@ LockAcquireExtended(const LOCKTAG *locktag,
/*
* LWLockAcquire acts as a memory sequencing point, so it's safe
* to assume that any strong locker whose increment to
- * FastPathStrongRelationLocks->counts becomes visible after we
- * test it has yet to begin to transfer fast-path locks.
+ * FastPathStrongRelationLocks becomes visible after we test it
+ * has yet to begin to transfer fast-path locks.
*/
LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE);
- if (FastPathStrongRelationLocks->count[fasthashcode] != 0)
+ if (pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) != 0)
acquired = false;
else
acquired = FastPathGrantRelationLock(locktag->locktag_field2,
@@ -1501,11 +1497,9 @@ RemoveLocalLock(LOCALLOCK *locallock)
fasthashcode = FastPathStrongLockHashPartition(locallock->hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = false;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
if (!hash_search(LockMethodLocalHash,
@@ -1834,20 +1828,9 @@ BeginStrongLockAcquire(LOCALLOCK *locallock, uint32 fasthashcode)
Assert(StrongLockInProgress == NULL);
Assert(locallock->holdsStrongLockCount == false);
- /*
- * Adding to a memory location is not atomic, so we take a spinlock to
- * ensure we don't collide with someone else trying to bump the count at
- * the same time.
- *
- * XXX: It might be worth considering using an atomic fetch-and-add
- * instruction here, on architectures where that is supported.
- */
-
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- FastPathStrongRelationLocks->count[fasthashcode]++;
+ pg_atomic_fetch_add_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = true;
StrongLockInProgress = locallock;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
/*
@@ -1875,12 +1858,10 @@ AbortStrongLockAcquire(void)
fasthashcode = FastPathStrongLockHashPartition(locallock->hashcode);
Assert(locallock->holdsStrongLockCount == true);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = false;
StrongLockInProgress = NULL;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
/*
@@ -3365,10 +3346,8 @@ LockRefindAndRelease(LockMethod lockMethodTable, PGPROC *proc,
{
uint32 fasthashcode = FastPathStrongLockHashPartition(hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
}
}
@@ -4504,9 +4483,7 @@ lock_twophase_recover(FullTransactionId fxid, uint16 info,
{
uint32 fasthashcode = FastPathStrongLockHashPartition(hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- FastPathStrongRelationLocks->count[fasthashcode]++;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
+ pg_atomic_fetch_add_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
}
LWLockRelease(partitionLock);
--
2.50.1 (Apple Git-155)
--Ot5x3ffkWEuxilWO--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v1 8/8] convert FastPathStrongRelationLocks to atomics
@ 2026-07-09 20:38 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2026-07-09 20:38 UTC (permalink / raw)
---
src/backend/storage/lmgr/lock.c | 57 ++++++++++-----------------------
1 file changed, 17 insertions(+), 40 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 5ee80c7632e..035932197a4 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -40,11 +40,11 @@
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
+#include "port/atomics.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
#include "storage/shmem.h"
-#include "storage/spin.h"
#include "storage/standby.h"
#include "storage/subsystems.h"
#include "utils/memutils.h"
@@ -306,13 +306,7 @@ static PROCLOCK *FastPathGetRelationLockEntry(LOCALLOCK *locallock);
#define FastPathStrongLockHashPartition(hashcode) \
((hashcode) % FAST_PATH_STRONG_LOCK_HASH_PARTITIONS)
-typedef struct
-{
- slock_t mutex;
- uint32 count[FAST_PATH_STRONG_LOCK_HASH_PARTITIONS];
-} FastPathStrongRelationLockData;
-
-static FastPathStrongRelationLockData *FastPathStrongRelationLocks;
+static pg_atomic_uint32 *FastPathStrongRelationLocks;
static void LockManagerShmemRequest(void *arg);
static void LockManagerShmemInit(void *arg);
@@ -484,7 +478,8 @@ LockManagerShmemRequest(void *arg)
);
ShmemRequestStruct(.name = "Fast Path Strong Relation Lock Data",
- .size = sizeof(FastPathStrongRelationLockData),
+ .size = mul_size(sizeof(pg_atomic_uint32),
+ FAST_PATH_STRONG_LOCK_HASH_PARTITIONS),
.ptr = (void **) (void *) &FastPathStrongRelationLocks,
);
}
@@ -492,7 +487,8 @@ LockManagerShmemRequest(void *arg)
static void
LockManagerShmemInit(void *arg)
{
- SpinLockInit(&FastPathStrongRelationLocks->mutex);
+ for (int i = 0; i < FAST_PATH_STRONG_LOCK_HASH_PARTITIONS; i++)
+ pg_atomic_init_u32(&FastPathStrongRelationLocks[i], 0);
}
/*
@@ -992,11 +988,11 @@ LockAcquireExtended(const LOCKTAG *locktag,
/*
* LWLockAcquire acts as a memory sequencing point, so it's safe
* to assume that any strong locker whose increment to
- * FastPathStrongRelationLocks->counts becomes visible after we
- * test it has yet to begin to transfer fast-path locks.
+ * FastPathStrongRelationLocks becomes visible after we test it
+ * has yet to begin to transfer fast-path locks.
*/
LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE);
- if (FastPathStrongRelationLocks->count[fasthashcode] != 0)
+ if (pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) != 0)
acquired = false;
else
acquired = FastPathGrantRelationLock(locktag->locktag_field2,
@@ -1501,11 +1497,9 @@ RemoveLocalLock(LOCALLOCK *locallock)
fasthashcode = FastPathStrongLockHashPartition(locallock->hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = false;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
if (!hash_search(LockMethodLocalHash,
@@ -1834,20 +1828,9 @@ BeginStrongLockAcquire(LOCALLOCK *locallock, uint32 fasthashcode)
Assert(StrongLockInProgress == NULL);
Assert(locallock->holdsStrongLockCount == false);
- /*
- * Adding to a memory location is not atomic, so we take a spinlock to
- * ensure we don't collide with someone else trying to bump the count at
- * the same time.
- *
- * XXX: It might be worth considering using an atomic fetch-and-add
- * instruction here, on architectures where that is supported.
- */
-
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- FastPathStrongRelationLocks->count[fasthashcode]++;
+ pg_atomic_fetch_add_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = true;
StrongLockInProgress = locallock;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
/*
@@ -1875,12 +1858,10 @@ AbortStrongLockAcquire(void)
fasthashcode = FastPathStrongLockHashPartition(locallock->hashcode);
Assert(locallock->holdsStrongLockCount == true);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
locallock->holdsStrongLockCount = false;
StrongLockInProgress = NULL;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
}
/*
@@ -3365,10 +3346,8 @@ LockRefindAndRelease(LockMethod lockMethodTable, PGPROC *proc,
{
uint32 fasthashcode = FastPathStrongLockHashPartition(hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- Assert(FastPathStrongRelationLocks->count[fasthashcode] > 0);
- FastPathStrongRelationLocks->count[fasthashcode]--;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
+ Assert(pg_atomic_read_u32(&FastPathStrongRelationLocks[fasthashcode]) > 0);
+ pg_atomic_fetch_sub_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
}
}
@@ -4504,9 +4483,7 @@ lock_twophase_recover(FullTransactionId fxid, uint16 info,
{
uint32 fasthashcode = FastPathStrongLockHashPartition(hashcode);
- SpinLockAcquire(&FastPathStrongRelationLocks->mutex);
- FastPathStrongRelationLocks->count[fasthashcode]++;
- SpinLockRelease(&FastPathStrongRelationLocks->mutex);
+ pg_atomic_fetch_add_u32(&FastPathStrongRelationLocks[fasthashcode], 1);
}
LWLockRelease(partitionLock);
--
2.50.1 (Apple Git-155)
--Ot5x3ffkWEuxilWO--
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2026-07-09 20:38 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-06-04 09:55 Re: Partitioned tables and [un]loggedness Shinya Kato <[email protected]>
2025-06-04 10:05 ` Shinya Kato <[email protected]>
2025-06-04 16:15 ` Nathan Bossart <[email protected]>
2025-06-05 00:23 ` Michael Paquier <[email protected]>
2025-06-05 01:04 ` Shinya Kato <[email protected]>
2025-06-05 04:57 ` Michael Paquier <[email protected]>
2025-06-05 20:04 ` Nathan Bossart <[email protected]>
2025-06-06 01:55 ` Michael Paquier <[email protected]>
2025-06-11 00:28 ` Michael Paquier <[email protected]>
2026-01-21 10:32 Re: Can we remove support for standard_conforming_strings = off yet? Julien Rouhaud <[email protected]>
2026-01-21 20:09 ` Re: Can we remove support for standard_conforming_strings = off yet? Tom Lane <[email protected]>
2026-01-21 20:25 ` Re: Can we remove support for standard_conforming_strings = off yet? Bruce Momjian <[email protected]>
2026-01-30 10:10 ` Re: Can we remove support for standard_conforming_strings = off yet? jian he <[email protected]>
2026-07-09 20:38 [PATCH v1 8/8] convert FastPathStrongRelationLocks to atomics Nathan Bossart <[email protected]>
2026-07-09 20:38 [PATCH v1 8/8] convert FastPathStrongRelationLocks to atomics Nathan Bossart <[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