agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH] Allow specifying access method of partitioned tables.. 5+ messages / 2 participants [nested] [flat]
* [PATCH] Allow specifying access method of partitioned tables.. @ 2021-03-07 06:11 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Justin Pryzby @ 2021-03-07 06:11 UTC (permalink / raw) ..to be inherited by partitions See also: ca4103025dfe26eaaf6a500dec9170fbb176eebc 8586bf7ed8889f39a59dd99b292014b73be85342 ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 - prevent DROP AM Authors: Justin Pryzby, Soumyadeep Chakraborty --- doc/src/sgml/ref/create_table.sgml | 9 ++- src/backend/catalog/heap.c | 3 +- src/backend/commands/tablecmds.c | 88 +++++++++++++++++++------ src/backend/utils/cache/lsyscache.c | 22 +++++++ src/backend/utils/cache/relcache.c | 4 ++ src/bin/pg_dump/pg_dump.c | 3 +- src/bin/pg_dump/t/002_pg_dump.pl | 16 ++++- src/include/utils/lsyscache.h | 1 + src/test/regress/expected/create_am.out | 45 +++++++++---- src/test/regress/sql/create_am.sql | 18 +++-- 10 files changed, 167 insertions(+), 42 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 10ef699fab9..b20d272b151 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -1295,9 +1295,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM This optional clause specifies the table access method to use to store the contents for the new table; the method needs be an access method of type <literal>TABLE</literal>. See <xref linkend="tableam"/> for more - information. If this option is not specified, the default table access - method is chosen for the new table. See <xref - linkend="guc-default-table-access-method"/> for more information. + information. If this option is not specified, a default table access + method is chosen for the new table. + When creating a partition, the default table access method is the + access method of its parent. + For other tables, the default is determined by + <xref linkend="guc-default-table-access-method"/>. </para> </listitem> </varlistentry> diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 2a0d82aedd7..bbf8e08618b 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1452,7 +1452,8 @@ heap_create_with_catalog(const char *relname, * No need to add an explicit dependency for the toast table, as the * main table depends on it. */ - if (RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE) + if ((RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE) || + relkind == RELKIND_PARTITIONED_TABLE) { ObjectAddressSet(referenced, AccessMethodRelationId, accessmtd); add_exact_object_address(&referenced, addrs); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3147dddf286..71b8b0b2317 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -571,6 +571,7 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode); static void ATExecDropCluster(Relation rel, LOCKMODE lockmode); static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname); +static void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod); static bool ATPrepChangePersistence(Relation rel, bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); @@ -680,7 +681,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, Oid ofTypeId; ObjectAddress address; LOCKMODE parentLockmode; - const char *accessMethod = NULL; Oid accessMethodId = InvalidOid; /* @@ -945,20 +945,21 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * a type of relation that needs one, use the default. */ if (stmt->accessMethod != NULL) + accessMethodId = get_table_am_oid(stmt->accessMethod, false); + else if (stmt->partbound && + (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)) { - accessMethod = stmt->accessMethod; - - if (partitioned) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("specifying a table access method is not supported on a partitioned table"))); + /* + * For partitions, if no access method is specified, default to the AM + * of the parent table. + */ + Assert(list_length(inheritOids) == 1); + accessMethodId = get_rel_relam(linitial_oid(inheritOids)); + if (!OidIsValid(accessMethodId)) + accessMethodId = get_table_am_oid(default_table_access_method, false); } - else if (RELKIND_HAS_TABLE_AM(relkind)) - accessMethod = default_table_access_method; - - /* look up the access method, verify it is for a table */ - if (accessMethod != NULL) - accessMethodId = get_table_am_oid(accessMethod, false); + else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE) + accessMethodId = get_table_am_oid(default_table_access_method, false); /* * Create the relation. Inherited defaults and constraints are passed in @@ -4787,12 +4788,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_SetAccessMethod: /* SET ACCESS METHOD */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW); - /* partitioned tables don't have an access method */ - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot change access method of a partitioned table"))); - /* check if another access method change was already requested */ if (OidIsValid(tab->newAccessMethod)) ereport(ERROR, @@ -5146,6 +5141,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetAccessMethod: /* SET ACCESS METHOD */ /* handled specially in Phase 3 */ + + /* + * Only do this for partitioned tables, for which this is just a + * catalog change. Tables with storage are handled by Phase 3. + */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod); break; case AT_SetTableSpace: /* SET TABLESPACE */ @@ -14541,6 +14543,54 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) list_free(reltoastidxids); } +/* + * Special handling of ALTER TABLE SET ACCESS METHOD for relations with no + * storage that have an interest in preserving AM. + * + * Since these have no storage, setting the access method is a catalog only + * operation. + */ +static void +ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod) +{ + Relation pg_class; + Oid relid; + Oid oldrelam; + HeapTuple tuple; + + /* + * Shouldn't be called on relations having storage; these are processed in + * phase 3. + */ + Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)); + + relid = RelationGetRelid(rel); + + /* Pull the record for this relation and update it */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid)); + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", relid); + + oldrelam = ((Form_pg_class) GETSTRUCT(tuple))->relam; + ((Form_pg_class) GETSTRUCT(tuple))->relam = newAccessMethod; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + /* Update dependency on new AM */ + changeDependencyFor(RelationRelationId, relid, AccessMethodRelationId, + oldrelam, newAccessMethod); + + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); + + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + + /* Make sure the relam change is visible */ + CommandCounterIncrement(); +} + /* * Special handling of ALTER TABLE SET TABLESPACE for relations with no * storage that have an interest in preserving tablespace. diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index c7607895cdd..39a97cf5d4c 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -2073,6 +2073,28 @@ get_rel_persistence(Oid relid) return result; } +/* + * get_rel_relam + * + * Returns the relam associated with a given relation. + */ +Oid +get_rel_relam(Oid relid) +{ + HeapTuple tp; + Form_pg_class reltup; + Oid result; + + tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tp)) + elog(ERROR, "cache lookup failed for relation %u", relid); + reltup = (Form_pg_class) GETSTRUCT(tp); + result = reltup->relam; + ReleaseSysCache(tp); + + return result; +} + /* ---------- TRANSFORM CACHE ---------- */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 40140de9589..08d5f315c87 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1207,6 +1207,10 @@ retry: else if (RELKIND_HAS_TABLE_AM(relation->rd_rel->relkind) || relation->rd_rel->relkind == RELKIND_SEQUENCE) RelationInitTableAccessMethod(relation); + else if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + /* Do nothing: it's a catalog settings for partitions to inherit */ + } else Assert(relation->rd_rel->relam == InvalidOid); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d62780a0880..c0dd029e36d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -16111,7 +16111,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) if (RELKIND_HAS_TABLESPACE(tbinfo->relkind)) tablespace = tbinfo->reltablespace; - if (RELKIND_HAS_TABLE_AM(tbinfo->relkind)) + if (RELKIND_HAS_TABLE_AM(tbinfo->relkind) || + tbinfo->relkind == RELKIND_PARTITIONED_TABLE) tableam = tbinfo->amname; ArchiveEntry(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 42215f82f7a..f1efff96d23 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -4380,13 +4380,25 @@ my %tests = ( create_sql => ' CREATE TABLE dump_test.regress_pg_dump_table_am_0() USING heap; CREATE TABLE dump_test.regress_pg_dump_table_am_1 (col1 int) USING regress_table_am; - CREATE TABLE dump_test.regress_pg_dump_table_am_2() USING heap;', + CREATE TABLE dump_test.regress_pg_dump_table_am_2() USING heap; + CREATE TABLE dump_test.regress_pg_dump_table_am_parent (id int) PARTITION BY RANGE (id) USING regress_table_am; + CREATE TABLE dump_test.regress_pg_dump_table_am_child PARTITION OF dump_test.regress_pg_dump_table_am_parent DEFAULT', regexp => qr/^ \QSET default_table_access_method = regress_table_am;\E (\n(?!SET[^;]+;)[^\n]*)* \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_1 (\E \n\s+\Qcol1 integer\E - \n\);/xm, + \n\); + (.*\n)* + \QSET default_table_access_method = heap;\E + (\n(?!SET[^;]+;)[^\n]*)* + \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_2 (\E + (.*\n)* + \QSET default_table_access_method = regress_table_am;\E + (\n(?!SET[^;]+;)[^\n]*)* + \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_parent (\E + (\n(?!SET[^;]+;)[^\n]*)* + \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child (\E/xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 4f5418b9728..9f26357d373 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -139,6 +139,7 @@ extern char get_rel_relkind(Oid relid); extern bool get_rel_relispartition(Oid relid); extern Oid get_rel_tablespace(Oid relid); extern char get_rel_persistence(Oid relid); +extern Oid get_rel_relam(Oid relid); extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes); extern Oid get_transform_tosql(Oid typid, Oid langid, List *trftypes); extern bool get_typisdefined(Oid typid); diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index e9a9283d7ab..34d86eacf65 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -176,15 +176,15 @@ SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1; 1 (1 row) --- CREATE TABLE .. PARTITION BY doesn't not support USING +-- CREATE TABLE .. PARTITION BY supports USING +-- new partitions will inherit the AM from their parent table CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; -ERROR: specifying a table access method is not supported on a partitioned table -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a); --- new partitions will inherit from the current default, rather the partition root SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); +CREATE TABLE tableam_parted_aa_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('aa') PARTITION BY LIST (a); SET default_table_access_method = 'heap2'; CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b'); +CREATE TABLE tableam_parted_bb_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('bb') PARTITION BY LIST (a); RESET default_table_access_method; -- but the method can be explicitly specified CREATE TABLE tableam_parted_c_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('c') USING heap; @@ -205,14 +205,19 @@ WHERE pa.oid = pc.relam ORDER BY 3, 1, 2; relkind | amname | relname ---------+--------+---------------------------------- + r | heap2 | tableam_parted_a_heap2 + p | heap2 | tableam_parted_aa_heap2 r | heap2 | tableam_parted_b_heap2 + p | heap2 | tableam_parted_bb_heap2 r | heap2 | tableam_parted_d_heap2 + p | heap2 | tableam_parted_heap2 r | heap2 | tableam_tbl_heap2 r | heap2 | tableam_tblas_heap2 m | heap2 | tableam_tblmv_heap2 + t | heap2 | toast for tableam_parted_a_heap2 t | heap2 | toast for tableam_parted_b_heap2 t | heap2 | toast for tableam_parted_d_heap2 -(7 rows) +(12 rows) -- Show dependencies onto AM - there shouldn't be any for toast SELECT pg_describe_object(classid,objid,objsubid) AS obj @@ -226,9 +231,13 @@ ORDER BY classid, objid, objsubid; table tableam_tbl_heap2 table tableam_tblas_heap2 materialized view tableam_tblmv_heap2 + table tableam_parted_heap2 + table tableam_parted_a_heap2 + table tableam_parted_aa_heap2 table tableam_parted_b_heap2 + table tableam_parted_bb_heap2 table tableam_parted_d_heap2 -(5 rows) +(9 rows) -- ALTER TABLE SET ACCESS METHOD CREATE TABLE heaptable USING heap AS @@ -284,11 +293,26 @@ ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2; ERROR: cannot have multiple SET ACCESS METHOD subcommands DROP MATERIALIZED VIEW heapmv; DROP TABLE heaptable; --- No support for partitioned tables. +-- partition hierarchies +-- upon ALTER, new children will inherit the new am, whereas the existing +-- children will remain untouched CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x); +CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 0); +CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 1); +ALTER TABLE am_partitioned_1 SET ACCESS METHOD heap2; ALTER TABLE am_partitioned SET ACCESS METHOD heap2; -ERROR: cannot change access method of a partitioned table +CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 2); +SELECT relname, amname FROM pg_class c, pg_am am + WHERE c.relam = am.oid AND c.relname LIKE 'am_partitioned%' ORDER BY 1; + relname | amname +------------------+-------- + am_partitioned | heap2 + am_partitioned_1 | heap2 + am_partitioned_2 | heap + am_partitioned_3 | heap2 +(4 rows) + DROP TABLE am_partitioned; -- Second, create objects in the new AM by changing the default AM BEGIN; @@ -326,7 +350,7 @@ ORDER BY 3, 1, 2; f | | tableam_fdw_heapx r | heap2 | tableam_parted_1_heapx r | heap | tableam_parted_2_heapx - p | | tableam_parted_heapx + p | heap2 | tableam_parted_heapx S | | tableam_seq_heapx r | heap2 | tableam_tbl_heapx r | heap2 | tableam_tblas_heapx @@ -355,7 +379,6 @@ ERROR: cannot drop access method heap2 because other objects depend on it DETAIL: table tableam_tbl_heap2 depends on access method heap2 table tableam_tblas_heap2 depends on access method heap2 materialized view tableam_tblmv_heap2 depends on access method heap2 -table tableam_parted_b_heap2 depends on access method heap2 -table tableam_parted_d_heap2 depends on access method heap2 +table tableam_parted_heap2 depends on access method heap2 HINT: Use DROP ... CASCADE to drop the dependent objects too. -- we intentionally leave the objects created above alive, to verify pg_dump support diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql index 256884c9592..73cef0da4c8 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -124,15 +124,15 @@ CREATE SEQUENCE tableam_seq_heap2 USING heap2; CREATE MATERIALIZED VIEW tableam_tblmv_heap2 USING heap2 AS SELECT * FROM tableam_tbl_heap2; SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1; --- CREATE TABLE .. PARTITION BY doesn't not support USING +-- CREATE TABLE .. PARTITION BY supports USING +-- new partitions will inherit the AM from their parent table CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; - -CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a); --- new partitions will inherit from the current default, rather the partition root SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); +CREATE TABLE tableam_parted_aa_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('aa') PARTITION BY LIST (a); SET default_table_access_method = 'heap2'; CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b'); +CREATE TABLE tableam_parted_bb_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('bb') PARTITION BY LIST (a); RESET default_table_access_method; -- but the method can be explicitly specified CREATE TABLE tableam_parted_c_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('c') USING heap; @@ -183,10 +183,18 @@ ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2; ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2; DROP MATERIALIZED VIEW heapmv; DROP TABLE heaptable; --- No support for partitioned tables. +-- partition hierarchies +-- upon ALTER, new children will inherit the new am, whereas the existing +-- children will remain untouched CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x); +CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 0); +CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 1); +ALTER TABLE am_partitioned_1 SET ACCESS METHOD heap2; ALTER TABLE am_partitioned SET ACCESS METHOD heap2; +CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 2); +SELECT relname, amname FROM pg_class c, pg_am am + WHERE c.relam = am.oid AND c.relname LIKE 'am_partitioned%' ORDER BY 1; DROP TABLE am_partitioned; -- Second, create objects in the new AM by changing the default AM -- 2.34.1 --0rwxpeBZXMp06Zzu-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v20 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 206 +++++++++--------- src/include/storage/lwlock.h | 24 +- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 120 insertions(+), 173 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..b4636d86ca6 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; /* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,31 +395,28 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ numLocks += NumLWLocksForNamedTranches(); - /* Space for dynamic allocation counter, plus room for alignment. */ - size = sizeof(int) + LWLOCK_PADDED_SIZE; + /* Space for dynamic allocation counter. */ + size = MAXALIGN(sizeof(int)); - /* Space for the LWLock array. */ - size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); + /* Space for named tranches. */ + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); - /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + /* Space for the LWLock array, plus room for cache line alignment. */ + size = add_size(size, LWLOCK_PADDED_SIZE); + size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -432,7 +432,16 @@ CreateLWLocks(void) /* Initialize the dynamic-allocation counter for tranches */ LWLockCounter = (int *) ptr; *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; - ptr += sizeof(int); + ptr += MAXALIGN(sizeof(int)); + + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; @@ -441,11 +450,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +458,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +488,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +551,44 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* We use the ShmemLock spinlock to protect LWLockCounter */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +611,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +649,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -709,18 +692,23 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..3877aaa7f03 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --oZuO6jTSiLeu2qvQ Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v20-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v21 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 218 +++++++++--------- src/include/storage/lwlock.h | 25 +- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 131 insertions(+), 175 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..7bd9aaf8377 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; -/* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; +/* shared memory counter of registered tranches */ int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,31 +395,28 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ numLocks += NumLWLocksForNamedTranches(); - /* Space for dynamic allocation counter, plus room for alignment. */ - size = sizeof(int) + LWLOCK_PADDED_SIZE; + /* Space for dynamic allocation counter. */ + size = MAXALIGN(sizeof(int)); - /* Space for the LWLock array. */ - size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); + /* Space for named tranches. */ + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); - /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + /* Space for the LWLock array, plus room for cache line alignment. */ + size = add_size(size, LWLOCK_PADDED_SIZE); + size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -432,7 +432,16 @@ CreateLWLocks(void) /* Initialize the dynamic-allocation counter for tranches */ LWLockCounter = (int *) ptr; *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; - ptr += sizeof(int); + ptr += MAXALIGN(sizeof(int)); + + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; @@ -441,11 +450,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +458,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +488,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +551,47 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* + * We use the ShmemLock spinlock to protect LWLockCounter and + * NamedLWLockTrancheNames. + */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +614,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +652,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -710,17 +696,29 @@ GetLWTrancheName(uint16 trancheId) return BuiltinTrancheNames[trancheId]; /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * We only ever add new entries to NamedLWLockTrancheNames, so most + * lookups can avoid taking the spinlock as long as the backend-local + * counter (LocalLWLockCounter) is greater than the requested tranche ID. + * Else, we need to first update the backend-local counter with ShmemLock + * held before attempting the lookup again. In practice, the latter case + * is probably rare. */ - trancheId -= LWTRANCHE_FIRST_USER_DEFINED; + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; + /* + * It's an extension tranche, so look in NamedLWLockTrancheNames. + */ + trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..16728521b9a 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,19 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); -extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); +extern int LWLockNewTrancheId(const char *name); extern void LWLockInitialize(LWLock *lock, int tranche_id); /* diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --+17XTdpDH5Q/lDP5 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v21-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v16 2/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 179 ++++++++---------- src/include/storage/lwlock.h | 24 +-- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 99 insertions(+), 167 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index bf6b55ee830..cbb44344b5a 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; PROC_HDR *ProcGlobal; @@ -760,7 +760,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; param->ProcGlobal = ProcGlobal; @@ -1020,7 +1020,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; ProcGlobal = param->ProcGlobal; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index 332796465ff..8999dee6761 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -377,8 +373,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -408,9 +403,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index c80b43f1f55..39f17c387f3 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,18 +185,18 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; -/* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); @@ -391,7 +392,6 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ @@ -404,18 +404,15 @@ LWLockShmemSize(void) size = add_size(size, sizeof(int) + LWLOCK_PADDED_SIZE); /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -429,6 +426,15 @@ CreateLWLocks(void) /* Allocate space */ ptr = (char *) ShmemAlloc(spaceLocks); + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } + /* Leave room for dynamic allocation of tranches */ ptr += sizeof(int); @@ -447,11 +453,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -460,7 +461,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -491,32 +491,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -568,61 +554,41 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; int *LWLockCounter; + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche name must be less than %d bytes.", + NAMEDATALEN))); + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + /* We use the ShmemLock spinlock to protect LWLockCounter */ SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); - - return result; -} - -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; - - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = LocalLWLockCounter = (*LWLockCounter)++; + strcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -647,22 +613,17 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); @@ -677,6 +638,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -717,18 +681,27 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + int *LWLockCounter; + + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 5e717765764..9f9c4c7b5ca 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; /* @@ -157,18 +150,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --PIgUBUNQ6mYWmpJO-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v19 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 193 ++++++++---------- src/include/storage/lwlock.h | 24 +-- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 113 insertions(+), 167 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..0e30dc1ba03 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; /* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,7 +395,6 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ @@ -405,18 +407,15 @@ LWLockShmemSize(void) size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -434,6 +433,15 @@ CreateLWLocks(void) *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; ptr += sizeof(int); + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } + /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; MainLWLockArray = (LWLockPadded *) ptr; @@ -441,11 +449,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +457,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +487,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +550,44 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* We use the ShmemLock spinlock to protect LWLockCounter */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +610,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +648,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -709,18 +691,23 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..3877aaa7f03 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --gKvEFVVTDjQ/d7cm Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v19-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2025-08-27 20:13 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-07 06:11 [PATCH] Allow specifying access method of partitioned tables.. Justin Pryzby <[email protected]> 2025-08-27 20:13 [PATCH v20 1/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v21 1/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v16 2/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v19 1/2] Move dynamically-allocated tranche names to shared memory. 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