public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v7 1/7] pg_dump: make CLUSTER ON a separate dump object..
8+ messages / 6 participants
[nested] [flat]
* [PATCH v7 1/7] pg_dump: make CLUSTER ON a separate dump object..
@ 2020-11-26 20:37 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Justin Pryzby @ 2020-11-26 20:37 UTC (permalink / raw)
..since it needs to be restored after any child indexes are restored *and
attached*. The order needs to be:
1) restore child and parent index (order doesn't matter);
2) attach child index;
3) set cluster on child and parent index (order doesn't matter);
---
src/bin/pg_dump/pg_dump.c | 86 ++++++++++++++++++++++++++--------
src/bin/pg_dump/pg_dump.h | 8 ++++
src/bin/pg_dump/pg_dump_sort.c | 8 ++++
3 files changed, 82 insertions(+), 20 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d99b61e621..8dc8a42964 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -208,6 +208,7 @@ static void dumpSequence(Archive *fout, TableInfo *tbinfo);
static void dumpSequenceData(Archive *fout, TableDataInfo *tdinfo);
static void dumpIndex(Archive *fout, IndxInfo *indxinfo);
static void dumpIndexAttach(Archive *fout, IndexAttachInfo *attachinfo);
+static void dumpIndexClusterOn(Archive *fout, IndexClusterInfo *clusterinfo);
static void dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo);
static void dumpConstraint(Archive *fout, ConstraintInfo *coninfo);
static void dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo);
@@ -7092,6 +7093,11 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
i_inddependcollversions;
int ntups;
+ int ncluster = 0;
+ IndexClusterInfo *clusterinfo;
+ clusterinfo = (IndexClusterInfo *)
+ pg_malloc0(numTables * sizeof(IndexClusterInfo));
+
for (i = 0; i < numTables; i++)
{
TableInfo *tbinfo = &tblinfo[i];
@@ -7471,6 +7477,27 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
/* Plain secondary index */
indxinfo[j].indexconstraint = 0;
}
+
+ /* Record each table's CLUSTERed index, if any */
+ if (indxinfo[j].indisclustered)
+ {
+ IndxInfo *index = &indxinfo[j];
+ IndexClusterInfo *cluster = &clusterinfo[ncluster];
+
+ cluster->dobj.objType = DO_INDEX_CLUSTER_ON;
+ cluster->dobj.catId.tableoid = 0;
+ cluster->dobj.catId.oid = 0;
+ AssignDumpId(&cluster->dobj);
+ cluster->dobj.name = pg_strdup(index->dobj.name);
+ cluster->dobj.namespace = index->indextable->dobj.namespace;
+ cluster->index = index;
+ cluster->indextable = &tblinfo[i];
+
+ /* The CLUSTER ON depends on its index.. */
+ addObjectDependency(&cluster->dobj, index->dobj.dumpId);
+
+ ncluster++;
+ }
}
PQclear(res);
@@ -10323,6 +10350,9 @@ dumpDumpableObject(Archive *fout, DumpableObject *dobj)
case DO_SUBSCRIPTION:
dumpSubscription(fout, (SubscriptionInfo *) dobj);
break;
+ case DO_INDEX_CLUSTER_ON:
+ dumpIndexClusterOn(fout, (IndexClusterInfo *) dobj);
+ break;
case DO_PRE_DATA_BOUNDARY:
case DO_POST_DATA_BOUNDARY:
/* never dumped, nothing to do */
@@ -16543,6 +16573,41 @@ getAttrName(int attrnum, TableInfo *tblInfo)
return NULL; /* keep compiler quiet */
}
+/*
+ * dumpIndexClusterOn
+ * record that the index is clustered.
+ */
+static void
+dumpIndexClusterOn(Archive *fout, IndexClusterInfo *clusterinfo)
+{
+ DumpOptions *dopt = fout->dopt;
+ TableInfo *tbinfo = clusterinfo->indextable;
+ char *qindxname;
+ PQExpBuffer q;
+
+ if (dopt->dataOnly)
+ return;
+
+ q = createPQExpBuffer();
+ qindxname = pg_strdup(fmtId(clusterinfo->dobj.name));
+
+ /* index name is not qualified in this syntax */
+ appendPQExpBuffer(q, "\nALTER TABLE %s CLUSTER ON %s;\n",
+ fmtQualifiedDumpable(tbinfo), qindxname);
+
+ if (clusterinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
+ ArchiveEntry(fout, clusterinfo->dobj.catId, clusterinfo->dobj.dumpId,
+ ARCHIVE_OPTS(.tag = clusterinfo->dobj.name,
+ .namespace = tbinfo->dobj.namespace->dobj.name,
+ .owner = tbinfo->rolname,
+ .description = "INDEX CLUSTER ON",
+ .section = SECTION_POST_DATA,
+ .createStmt = q->data));
+
+ destroyPQExpBuffer(q);
+ free(qindxname);
+}
+
/*
* dumpIndex
* write out to fout a user-defined index
@@ -16597,16 +16662,6 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
* similar code in dumpConstraint!
*/
- /* If the index is clustered, we need to record that. */
- if (indxinfo->indisclustered)
- {
- appendPQExpBuffer(q, "\nALTER TABLE %s CLUSTER",
- fmtQualifiedDumpable(tbinfo));
- /* index name is not qualified in this syntax */
- appendPQExpBuffer(q, " ON %s;\n",
- qindxname);
- }
-
/*
* If the index has any statistics on some of its columns, generate
* the associated ALTER INDEX queries.
@@ -16933,16 +16988,6 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
* similar code in dumpIndex!
*/
- /* If the index is clustered, we need to record that. */
- if (indxinfo->indisclustered)
- {
- appendPQExpBuffer(q, "\nALTER TABLE %s CLUSTER",
- fmtQualifiedDumpable(tbinfo));
- /* index name is not qualified in this syntax */
- appendPQExpBuffer(q, " ON %s;\n",
- fmtId(indxinfo->dobj.name));
- }
-
/* If the index defines identity, we need to record that. */
if (indxinfo->indisreplident)
{
@@ -18448,6 +18493,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
break;
case DO_INDEX:
case DO_INDEX_ATTACH:
+ case DO_INDEX_CLUSTER_ON:
case DO_STATSEXT:
case DO_REFRESH_MATVIEW:
case DO_TRIGGER:
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 1290f9659b..57def4c009 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -54,6 +54,7 @@ typedef enum
DO_ATTRDEF,
DO_INDEX,
DO_INDEX_ATTACH,
+ DO_INDEX_CLUSTER_ON,
DO_STATSEXT,
DO_RULE,
DO_TRIGGER,
@@ -386,6 +387,13 @@ typedef struct _indxInfo
DumpId indexconstraint;
} IndxInfo;
+typedef struct _indexClusterInfo
+{
+ DumpableObject dobj;
+ TableInfo *indextable; /* link to table the index is for */
+ IndxInfo *index; /* link to index itself */
+} IndexClusterInfo;
+
typedef struct _indexAttachInfo
{
DumpableObject dobj;
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 46461fb6a1..dd5b233196 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -75,6 +75,7 @@ enum dbObjectTypePriorities
PRIO_CONSTRAINT,
PRIO_INDEX,
PRIO_INDEX_ATTACH,
+ PRIO_INDEX_CLUSTER_ON,
PRIO_STATSEXT,
PRIO_RULE,
PRIO_TRIGGER,
@@ -108,6 +109,7 @@ static const int dbObjectTypePriority[] =
PRIO_ATTRDEF, /* DO_ATTRDEF */
PRIO_INDEX, /* DO_INDEX */
PRIO_INDEX_ATTACH, /* DO_INDEX_ATTACH */
+ PRIO_INDEX_CLUSTER_ON, /* DO_INDEX_CLUSTER_ON */
PRIO_STATSEXT, /* DO_STATSEXT */
PRIO_RULE, /* DO_RULE */
PRIO_TRIGGER, /* DO_TRIGGER */
@@ -136,6 +138,7 @@ static const int dbObjectTypePriority[] =
PRIO_PUBLICATION, /* DO_PUBLICATION */
PRIO_PUBLICATION_REL, /* DO_PUBLICATION_REL */
PRIO_SUBSCRIPTION /* DO_SUBSCRIPTION */
+
};
StaticAssertDecl(lengthof(dbObjectTypePriority) == (DO_SUBSCRIPTION + 1),
@@ -1348,6 +1351,11 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
"INDEX ATTACH %s (ID %d)",
obj->name, obj->dumpId);
return;
+ case DO_INDEX_CLUSTER_ON:
+ snprintf(buf, bufsize,
+ "INDEX CLUSTER ON %s (ID %d)",
+ obj->name, obj->dumpId);
+ return;
case DO_STATSEXT:
snprintf(buf, bufsize,
"STATISTICS %s (ID %d OID %u)",
--
2.17.0
--AA9g+nFNFPYNJKiL
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0002-Implement-CLUSTER-of-partitioned-table.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
@ 2023-02-22 12:40 [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: [email protected] @ 2023-02-22 12:40 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected] <[email protected]>
On Wed, Feb 22, 2023 at 8:37 AM Nathan Bossart <[email protected]> wrote:
> On Wed, Feb 15, 2023 at 08:16:43AM +0000, [email protected] wrote:
> > When I refer to the GUC "max_locks_per_transaction", I find that the
> description
> > of the shared lock table size in pg-doc[1] is inconsistent with the code
> > (guc_table.c). BTW, the GUC "max_predicate_locks_per_xact" has similar
> problems.
> >
> > I think the descriptions in pg-doc are correct.
> > - GUC "max_locks_per_transaction"
> > Please refer to the macro "NLOCKENTS" used to obtain max_table_size in the
> > function InitLocks.
> >
> > - GUC "max_predicate_locks_per_xact"
> > Please refer to the macro "NPREDICATELOCKTARGETENTS" used to obtain
> > max_table_size in the function InitPredicateLocks.
>
> The GUC description for max_locks_per_transaction was first added in
> b700a67 (July 2003). Neither the GUC description nor the documentation was
> updated when max_prepared_transactions was introduced in d0a8968 (July
> 2005). However, the documentation was later fixed via 78ef2d3 (August
> 2005). It looks like the GUC description for
> max_predicate_locks_per_transaction was wrong from the start. In dafaa3e
> (February 2011), the GUC description does not include
> max_prepared_transactions, but the documentation does.
>
> It's interesting that the documentation cites max_connections, as the
> tables are sized using MaxBackends, which includes more than just
> max_connections (e.g., autovacuum_max_workers, max_worker_processes,
> max_wal_senders). After some digging, I see that MaxBackends was the
> original variable used for max_connections (which was called max_backends
> until 648677c (July 2000)), and it wasn't until autovacuum_max_workers was
> introduced in e2a186b (April 2007) before max_connections got its own
> MaxConnections variable and started diverging from MaxBackends.
>
> So, even with your patch applied, I don't think the formulas are correct.
> I don't know if it's worth writing out the exact formula, though. It
> doesn't seem to be kept up-to-date, and I don't know if users would choose
> different values for max_locks_per_transaction if it _was_ updated.
> Perhaps max_connections is a good enough approximation of MaxBackends most
> of the time...
Thanks very much for your careful review.
Yes, you are right. I think the formulas in the v1 patch are all approximations.
I think the exact formula (see function InitializeMaxBackends) is:
```
max_locks_per_transaction * (max_connections + autovacuum_max_workers + 1 +
max_worker_processes + max_wal_senders +
max_prepared_transactions)
```
After some rethinking, I think users can easily get exact value according to
exact formula, and I think using accurate formula can help users adjust
max_locks_per_transaction or max_predicate_locks_per_transaction if needed. So,
I used the exact formulas in the attached v2 patch.
Regards,
Wang Wei
Attachments:
[application/octet-stream] v2-0001-Fix-the-description-of-shared-lock-table-size-and.patch (3.5K, ../../OSZPR01MB627839B978B5735773D864FD9EAA9@OSZPR01MB6278.jpnprd01.prod.outlook.com/2-v2-0001-Fix-the-description-of-shared-lock-table-size-and.patch)
download | inline diff:
From d1e51b531a80eb80a6d8ae17a78577ee98d641a0 Mon Sep 17 00:00:00 2001
From: Wang Wei <[email protected]>
Date: Wed, 15 Feb 2023 15:25:59 +0800
Subject: [PATCH v2] Fix the description of shared lock table size and shared
predicate lock table size
---
doc/src/sgml/config.sgml | 6 ++++++
src/backend/utils/misc/guc_tables.c | 10 ++++++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ecd9aa73ef..d134988dfc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10150,6 +10150,9 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
The shared lock table tracks locks on
<varname>max_locks_per_transaction</varname> * (<xref
linkend="guc-max-connections"/> + <xref
+ linkend="guc-autovacuum-max-workers"/> + 1 + <xref
+ linkend="guc-max-worker-processes"/> + <xref
+ linkend="guc-max-wal-senders"/> + <xref
linkend="guc-max-prepared-transactions"/>) objects (e.g., tables);
hence, no more than this many distinct objects can be locked at
any one time. This parameter controls the average number of object
@@ -10182,6 +10185,9 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
The shared predicate lock table tracks locks on
<varname>max_pred_locks_per_transaction</varname> * (<xref
linkend="guc-max-connections"/> + <xref
+ linkend="guc-autovacuum-max-workers"/> + 1 + <xref
+ linkend="guc-max-worker-processes"/> + <xref
+ linkend="guc-max-wal-senders"/> + <xref
linkend="guc-max-prepared-transactions"/>) objects (e.g., tables);
hence, no more than this many distinct objects can be locked at
any one time. This parameter controls the average number of object
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1c0583fe26..1460ca2f8f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2571,8 +2571,9 @@ struct config_int ConfigureNamesInt[] =
{"max_locks_per_transaction", PGC_POSTMASTER, LOCK_MANAGEMENT,
gettext_noop("Sets the maximum number of locks per transaction."),
gettext_noop("The shared lock table is sized on the assumption that "
- "at most max_locks_per_transaction * max_connections distinct "
- "objects will need to be locked at any one time.")
+ "at most max_locks_per_transaction * (max_connections + autovacuum_max_workers + 1 "
+ "+ max_worker_processes + max_wal_senders + max_prepared_transactions) "
+ "distinct objects will need to be locked at any one time.")
},
&max_locks_per_xact,
64, 10, INT_MAX,
@@ -2583,8 +2584,9 @@ struct config_int ConfigureNamesInt[] =
{"max_pred_locks_per_transaction", PGC_POSTMASTER, LOCK_MANAGEMENT,
gettext_noop("Sets the maximum number of predicate locks per transaction."),
gettext_noop("The shared predicate lock table is sized on the assumption that "
- "at most max_pred_locks_per_transaction * max_connections distinct "
- "objects will need to be locked at any one time.")
+ "at most max_pred_locks_per_transaction * (max_connections + autovacuum_max_workers + 1 "
+ "+ max_worker_processes + max_wal_senders + max_prepared_transactions) "
+ "distinct objects will need to be locked at any one time.")
},
&max_predicate_locks_per_xact,
64, 10, INT_MAX,
--
2.39.1.windows.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
@ 2023-02-22 17:07 ` Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Bossart @ 2023-02-22 17:07 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: [email protected] <[email protected]>
On Wed, Feb 22, 2023 at 12:40:07PM +0000, [email protected] wrote:
> On Wed, Feb 22, 2023 at 8:37 AM Nathan Bossart <[email protected]> wrote:
>> So, even with your patch applied, I don't think the formulas are correct.
>> I don't know if it's worth writing out the exact formula, though. It
>> doesn't seem to be kept up-to-date, and I don't know if users would choose
>> different values for max_locks_per_transaction if it _was_ updated.
>> Perhaps max_connections is a good enough approximation of MaxBackends most
>> of the time...
>
> Thanks very much for your careful review.
>
> Yes, you are right. I think the formulas in the v1 patch are all approximations.
> I think the exact formula (see function InitializeMaxBackends) is:
> ```
> max_locks_per_transaction * (max_connections + autovacuum_max_workers + 1 +
> max_worker_processes + max_wal_senders +
> max_prepared_transactions)
> ```
>
> After some rethinking, I think users can easily get exact value according to
> exact formula, and I think using accurate formula can help users adjust
> max_locks_per_transaction or max_predicate_locks_per_transaction if needed. So,
> I used the exact formulas in the attached v2 patch.
IMHO this is too verbose. Perhaps it could be simplified to something like
The shared lock table is sized on the assumption that at most
max_locks_per_transaction objects per eligible process or prepared
transaction will need to be locked at any one time.
But if others disagree and think the full formula is appropriate, I'm fine
with it.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
@ 2023-04-04 15:47 ` Tom Lane <[email protected]>
2023-04-07 08:46 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2023-04-04 15:47 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected] <[email protected]>; [email protected] <[email protected]>
Nathan Bossart <[email protected]> writes:
> On Wed, Feb 22, 2023 at 12:40:07PM +0000, [email protected] wrote:
>> After some rethinking, I think users can easily get exact value according to
>> exact formula, and I think using accurate formula can help users adjust
>> max_locks_per_transaction or max_predicate_locks_per_transaction if needed. So,
>> I used the exact formulas in the attached v2 patch.
> IMHO this is too verbose.
Yeah, it's impossibly verbose. Even the current wording does not fit
nicely in pg_settings output.
> Perhaps it could be simplified to something like
> The shared lock table is sized on the assumption that at most
> max_locks_per_transaction objects per eligible process or prepared
> transaction will need to be locked at any one time.
I like the "per eligible process" wording, at least for guc_tables.c;
or maybe it could be "per server process"? That would be more
accurate and not much longer than what we have now.
I've got mixed emotions about trying to put the exact formulas into
the SGML docs either. Space isn't such a constraint there, but I
think the info would soon go out of date (indeed, I think the existing
wording was once exactly accurate), and I'm not sure it's worth trying
to maintain it precisely.
One reason that I'm not very excited about this is that in fact the
formula seen in the source code is not exact either; it's a lower
bound for how much space will be available. That's because we throw
in 100K slop at the bottom of the shmem sizing calculation, and a
large chunk of that remains available to be eaten by the lock table
if necessary.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
@ 2023-04-07 08:46 ` [email protected] <[email protected]>
2023-04-07 17:32 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: [email protected] @ 2023-04-07 08:46 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; +Cc: [email protected] <[email protected]>
On Tues, Apr 4, 2023 at 23:48 PM Tom Lane <[email protected]> wrote:
> Nathan Bossart <[email protected]> writes:
> > On Wed, Feb 22, 2023 at 12:40:07PM +0000, [email protected] wrote:
> >> After some rethinking, I think users can easily get exact value according to
> >> exact formula, and I think using accurate formula can help users adjust
> >> max_locks_per_transaction or max_predicate_locks_per_transaction if
> needed. So,
> >> I used the exact formulas in the attached v2 patch.
>
> > IMHO this is too verbose.
>
> Yeah, it's impossibly verbose. Even the current wording does not fit
> nicely in pg_settings output.
>
> > Perhaps it could be simplified to something like
> > The shared lock table is sized on the assumption that at most
> > max_locks_per_transaction objects per eligible process or prepared
> > transaction will need to be locked at any one time.
>
> I like the "per eligible process" wording, at least for guc_tables.c;
> or maybe it could be "per server process"? That would be more
> accurate and not much longer than what we have now.
>
> I've got mixed emotions about trying to put the exact formulas into
> the SGML docs either. Space isn't such a constraint there, but I
> think the info would soon go out of date (indeed, I think the existing
> wording was once exactly accurate), and I'm not sure it's worth trying
> to maintain it precisely.
Thanks both for sharing your opinions.
I agree that verbose descriptions make maintenance difficult.
For consistency, I unified the formulas in guc_tables.c and pg-doc into the same
suggested short formula. Attach the new patch.
> One reason that I'm not very excited about this is that in fact the
> formula seen in the source code is not exact either; it's a lower
> bound for how much space will be available. That's because we throw
> in 100K slop at the bottom of the shmem sizing calculation, and a
> large chunk of that remains available to be eaten by the lock table
> if necessary.
Thanks for sharing this.
Since no one has reported related issues, I'm also fine to close this entry if
this related modification is not necessary.
Regards,
Wang Wei
Attachments:
[application/octet-stream] v3-0001-Fix-the-description-of-shared-lock-table-size-and.patch (6.0K, ../../OS3PR01MB627541DAD068D37FA823839D9E969@OS3PR01MB6275.jpnprd01.prod.outlook.com/2-v3-0001-Fix-the-description-of-shared-lock-table-size-and.patch)
download | inline diff:
From fdd3849e3b9ca039c2a3cee4cdd0a443cca316e8 Mon Sep 17 00:00:00 2001
From: Wang Wei <[email protected]>
Date: Wed, 15 Feb 2023 15:25:59 +0800
Subject: [PATCH v3] Fix the description of shared lock table size and shared
predicate lock table size
---
doc/src/sgml/config.sgml | 49 ++++++++++++++---------------
src/backend/utils/misc/guc_tables.c | 10 +++---
2 files changed, 28 insertions(+), 31 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 25111d5caf..c3ebacdf54 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10237,19 +10237,18 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
<listitem>
<para>
The shared lock table tracks locks on
- <varname>max_locks_per_transaction</varname> * (<xref
- linkend="guc-max-connections"/> + <xref
- linkend="guc-max-prepared-transactions"/>) objects (e.g., tables);
- hence, no more than this many distinct objects can be locked at
- any one time. This parameter controls the average number of object
- locks allocated for each transaction; individual transactions
- can lock more objects as long as the locks of all transactions
- fit in the lock table. This is <emphasis>not</emphasis> the number of
- rows that can be locked; that value is unlimited. The default,
- 64, has historically proven sufficient, but you might need to
- raise this value if you have queries that touch many different
- tables in a single transaction, e.g., query of a parent table with
- many children. This parameter can only be set at server start.
+ <varname>max_locks_per_transaction</varname> objects (e.g., tables)
+ per eligible process or prepared transaction; hence, no more than this
+ many distinct objects can be locked at any one time. This parameter
+ controls the average number of object locks allocated for each
+ transaction; individual transactions can lock more objects as long as
+ the locks of all transactions fit in the lock table. This is
+ <emphasis>not</emphasis> the number of rows that can be locked; that
+ value is unlimited. The default, 64, has historically proven
+ sufficient, but you might need to raise this value if you have queries
+ that touch many different tables in a single transaction, e.g., query
+ of a parent table with many children. This parameter can only be set
+ at server start.
</para>
<para>
@@ -10269,19 +10268,17 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
<listitem>
<para>
The shared predicate lock table tracks locks on
- <varname>max_pred_locks_per_transaction</varname> * (<xref
- linkend="guc-max-connections"/> + <xref
- linkend="guc-max-prepared-transactions"/>) objects (e.g., tables);
- hence, no more than this many distinct objects can be locked at
- any one time. This parameter controls the average number of object
- locks allocated for each transaction; individual transactions
- can lock more objects as long as the locks of all transactions
- fit in the lock table. This is <emphasis>not</emphasis> the number of
- rows that can be locked; that value is unlimited. The default,
- 64, has generally been sufficient in testing, but you might need to
- raise this value if you have clients that touch many different
- tables in a single serializable transaction. This parameter can
- only be set at server start.
+ <varname>max_pred_locks_per_transaction</varname> objects (e.g.,
+ tables) per eligible process or prepared transaction; hence, no more
+ than this many distinct objects can be locked at any one time. This
+ parameter controls the average number of object locks allocated for
+ each transaction; individual transactions can lock more objects as
+ long as the locks of all transactions fit in the lock table. This is
+ <emphasis>not</emphasis> the number of rows that can be locked; that
+ value is unlimited. The default, 64, has generally been sufficient in
+ testing, but you might need to raise this value if you have clients
+ that touch many different tables in a single serializable transaction.
+ This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index e8e8245e91..c07d2a8775 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2599,9 +2599,9 @@ struct config_int ConfigureNamesInt[] =
{
{"max_locks_per_transaction", PGC_POSTMASTER, LOCK_MANAGEMENT,
gettext_noop("Sets the maximum number of locks per transaction."),
- gettext_noop("The shared lock table is sized on the assumption that "
- "at most max_locks_per_transaction * max_connections distinct "
- "objects will need to be locked at any one time.")
+ gettext_noop("The shared lock table is sized on the assumption that at most "
+ "max_locks_per_transaction objects per eligible process or prepared "
+ "transaction will need to be locked at any one time.")
},
&max_locks_per_xact,
64, 10, INT_MAX,
@@ -2612,8 +2612,8 @@ struct config_int ConfigureNamesInt[] =
{"max_pred_locks_per_transaction", PGC_POSTMASTER, LOCK_MANAGEMENT,
gettext_noop("Sets the maximum number of predicate locks per transaction."),
gettext_noop("The shared predicate lock table is sized on the assumption that "
- "at most max_pred_locks_per_transaction * max_connections distinct "
- "objects will need to be locked at any one time.")
+ "at most max_pred_locks_per_transaction objects per eligible process "
+ "or prepared transaction will need to be locked at any one time.")
},
&max_predicate_locks_per_xact,
64, 10, INT_MAX,
--
2.39.1.windows.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
2023-04-07 08:46 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
@ 2023-04-07 17:32 ` Tom Lane <[email protected]>
2023-04-10 01:24 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Wei Wang (Fujitsu) <[email protected]>
2024-04-26 11:40 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Michael Banck <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Tom Lane @ 2023-04-07 17:32 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected] <[email protected]>
"[email protected]" <[email protected]> writes:
> On Tues, Apr 4, 2023 at 23:48 PM Tom Lane <[email protected]> wrote:
>> I like the "per eligible process" wording, at least for guc_tables.c;
>> or maybe it could be "per server process"? That would be more
>> accurate and not much longer than what we have now.
> Thanks both for sharing your opinions.
> I agree that verbose descriptions make maintenance difficult.
> For consistency, I unified the formulas in guc_tables.c and pg-doc into the same
> suggested short formula. Attach the new patch.
After studying this for awhile, I decided "server process" is probably
the better term --- people will have some idea what that means, while
"eligible process" is not a term we use anywhere else. Pushed with
that change and some minor other wordsmithing.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
2023-04-07 08:46 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-04-07 17:32 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
@ 2023-04-10 01:24 ` Wei Wang (Fujitsu) <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Wei Wang (Fujitsu) @ 2023-04-10 01:24 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected] <[email protected]>
On Sat, Apr 8, 2023 at 1:32 AM Tom Lane <[email protected]> wrote:
> "[email protected]" <[email protected]> writes:
> > On Tues, Apr 4, 2023 at 23:48 PM Tom Lane <[email protected]> wrote:
> >> I like the "per eligible process" wording, at least for guc_tables.c;
> >> or maybe it could be "per server process"? That would be more
> >> accurate and not much longer than what we have now.
>
> > Thanks both for sharing your opinions.
> > I agree that verbose descriptions make maintenance difficult.
> > For consistency, I unified the formulas in guc_tables.c and pg-doc into the same
> > suggested short formula. Attach the new patch.
>
> After studying this for awhile, I decided "server process" is probably
> the better term --- people will have some idea what that means, while
> "eligible process" is not a term we use anywhere else. Pushed with
> that change and some minor other wordsmithing.
Make sense to me
Thanks for pushing.
Regards,
Wang Wei
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
2023-04-07 08:46 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-04-07 17:32 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
@ 2024-04-26 11:40 ` Michael Banck <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Michael Banck @ 2024-04-26 11:40 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected] <[email protected]>; Nathan Bossart <[email protected]>; [email protected] <[email protected]>
Hi,
On Fri, Apr 07, 2023 at 01:32:22PM -0400, Tom Lane wrote:
> "[email protected]" <[email protected]> writes:
> > On Tues, Apr 4, 2023 at 23:48 PM Tom Lane <[email protected]> wrote:
> >> I like the "per eligible process" wording, at least for guc_tables.c;
> >> or maybe it could be "per server process"? That would be more
> >> accurate and not much longer than what we have now.
>
> > Thanks both for sharing your opinions.
> > I agree that verbose descriptions make maintenance difficult.
> > For consistency, I unified the formulas in guc_tables.c and pg-doc into the same
> > suggested short formula. Attach the new patch.
>
> After studying this for awhile, I decided "server process" is probably
> the better term --- people will have some idea what that means, while
> "eligible process" is not a term we use anywhere else. Pushed with
> that change and some minor other wordsmithing.
I stumbled upon this change while looking at the documentation searching
for guidance and what max_locks_per_transactions should be set to (or
rather, a pointer about max_locks_per_transactions not actually being
"per transaction", but a shared pool of roughly
max_locks_per_transactions * max_connections).
While I agree that the exact formula is too verbose, I find the current
wording ("per server process or prepared transaction") to be misleading;
I can see how somebody sees that as a dynamic limit based on the current
number of running server processes or prepared transactions, not
something that is allocated at server start based on some hardcoded
GUCs.
I don't have a good alternative wording for now, but I wanted to point
out that currently the wording does not seem to imply
max_{connection,prepared_transactions} being at play at all. Probably
the GUC description cannot be made much clearer without making it too
verbose, but I think the description in config.sgml has more leeway to
get a mention of max_connections back.
Michael
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2024-04-26 11:40 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 20:37 [PATCH v7 1/7] pg_dump: make CLUSTER ON a separate dump object.. Justin Pryzby <[email protected]>
2023-02-22 12:40 RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-02-22 17:07 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Nathan Bossart <[email protected]>
2023-04-04 15:47 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
2023-04-07 08:46 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c [email protected] <[email protected]>
2023-04-07 17:32 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Tom Lane <[email protected]>
2023-04-10 01:24 ` RE: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Wei Wang (Fujitsu) <[email protected]>
2024-04-26 11:40 ` Re: Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c Michael Banck <[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