agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] unsuitable relkind
7+ messages / 5 participants
[nested] [flat]
* [PATCH v2] unsuitable relkind
@ 2018-12-19 17:33 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Alvaro Herrera @ 2018-12-19 17:33 UTC (permalink / raw)
---
.../pg_visibility/expected/pg_visibility.out | 75 ++++++++++++-------
contrib/pg_visibility/pg_visibility.c | 7 +-
contrib/pgstattuple/expected/pgstattuple.out | 9 ++-
contrib/pgstattuple/pgstatindex.c | 13 ++--
src/backend/catalog/catalog.c | 45 +++++++++++
src/backend/catalog/toasting.c | 6 +-
src/backend/commands/comment.c | 7 +-
src/backend/commands/seclabel.c | 6 +-
src/include/catalog/catalog.h | 2 +
9 files changed, 127 insertions(+), 43 deletions(-)
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out
index f0dcb897c4..d72ea2f63e 100644
--- a/contrib/pg_visibility/expected/pg_visibility.out
+++ b/contrib/pg_visibility/expected/pg_visibility.out
@@ -6,66 +6,91 @@ CREATE EXTENSION pg_visibility;
create table test_partitioned (a int) partition by list (a);
-- these should all fail
select pg_visibility('test_partitioned', 0);
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
select pg_visibility_map('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
select pg_visibility_map_summary('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
select pg_check_frozen('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
select pg_truncate_visibility_map('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
create table test_partition partition of test_partitioned for values in (1);
create index test_index on test_partition (a);
-- indexes do not, so these all fail
select pg_visibility('test_index', 0);
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_index"
+DETAIL: "test_index" is an index.
select pg_visibility_map('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_index"
+DETAIL: "test_index" is an index.
select pg_visibility_map_summary('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_index"
+DETAIL: "test_index" is an index.
select pg_check_frozen('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_index"
+DETAIL: "test_index" is an index.
select pg_truncate_visibility_map('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_index"
+DETAIL: "test_index" is an index.
create view test_view as select 1;
-- views do not have VMs, so these all fail
select pg_visibility('test_view', 0);
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_view"
+DETAIL: "test_view" is a view.
select pg_visibility_map('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_view"
+DETAIL: "test_view" is a view.
select pg_visibility_map_summary('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_view"
+DETAIL: "test_view" is a view.
select pg_check_frozen('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_view"
+DETAIL: "test_view" is a view.
select pg_truncate_visibility_map('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_view"
+DETAIL: "test_view" is a view.
create sequence test_sequence;
-- sequences do not have VMs, so these all fail
select pg_visibility('test_sequence', 0);
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_sequence"
+DETAIL: "test_sequence" is a sequence.
select pg_visibility_map('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_sequence"
+DETAIL: "test_sequence" is a sequence.
select pg_visibility_map_summary('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_sequence"
+DETAIL: "test_sequence" is a sequence.
select pg_check_frozen('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_sequence"
+DETAIL: "test_sequence" is a sequence.
select pg_truncate_visibility_map('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_sequence"
+DETAIL: "test_sequence" is a sequence.
create foreign data wrapper dummy;
create server dummy_server foreign data wrapper dummy;
create foreign table test_foreign_table () server dummy_server;
-- foreign tables do not have VMs, so these all fail
select pg_visibility('test_foreign_table', 0);
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
select pg_visibility_map('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
select pg_visibility_map_summary('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
select pg_check_frozen('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
select pg_truncate_visibility_map('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot examine visibility information for "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
-- check some of the allowed relkinds
create table regular_table (a int);
insert into regular_table values (1), (2);
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 3282742b80..24f164f8e8 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -13,6 +13,7 @@
#include "access/heapam.h"
#include "access/htup_details.h"
#include "access/visibilitymap.h"
+#include "catalog/catalog.h"
#include "catalog/pg_type.h"
#include "catalog/storage_xlog.h"
#include "funcapi.h"
@@ -776,6 +777,8 @@ check_relation_relkind(Relation rel)
rel->rd_rel->relkind != RELKIND_TOASTVALUE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, materialized view, or TOAST table",
- RelationGetRelationName(rel))));
+ errmsg("cannot examine visibility information for \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_unsuitable_relkind(rel->rd_rel->relkind,
+ RelationGetRelationName(rel))));
}
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 9858ea69d4..753087cdc8 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -161,7 +161,8 @@ ERROR: "test_partitioned_index" (partitioned index) is not supported
select pgstattuple_approx('test_partitioned');
ERROR: "test_partitioned" is not a table or materialized view
select pg_relpages('test_partitioned');
-ERROR: "test_partitioned" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot compute relpages of "test_partitioned"
+DETAIL: "test_partitioned" is a partitioned table.
select pgstatindex('test_partitioned');
ERROR: relation "test_partitioned" is not a btree index
select pgstatginindex('test_partitioned');
@@ -175,7 +176,8 @@ ERROR: "test_view" (view) is not supported
select pgstattuple_approx('test_view');
ERROR: "test_view" is not a table or materialized view
select pg_relpages('test_view');
-ERROR: "test_view" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot compute relpages of "test_view"
+DETAIL: "test_view" is a view.
select pgstatindex('test_view');
ERROR: relation "test_view" is not a btree index
select pgstatginindex('test_view');
@@ -191,7 +193,8 @@ ERROR: "test_foreign_table" (foreign table) is not supported
select pgstattuple_approx('test_foreign_table');
ERROR: "test_foreign_table" is not a table or materialized view
select pg_relpages('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot compute relpages of "test_foreign_table"
+DETAIL: "test_foreign_table" is a foreign table.
select pgstatindex('test_foreign_table');
ERROR: relation "test_foreign_table" is not a btree index
select pgstatginindex('test_foreign_table');
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 2c80753726..02dcf77287 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -33,6 +33,7 @@
#include "access/nbtree.h"
#include "access/relation.h"
#include "access/table.h"
+#include "catalog/catalog.h"
#include "catalog/namespace.h"
#include "catalog/pg_am.h"
#include "funcapi.h"
@@ -758,13 +759,11 @@ GetHashPageStats(Page page, HashIndexStat *stats)
static void
check_relation_relkind(Relation rel)
{
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_INDEX &&
- rel->rd_rel->relkind != RELKIND_MATVIEW &&
- rel->rd_rel->relkind != RELKIND_SEQUENCE &&
- rel->rd_rel->relkind != RELKIND_TOASTVALUE)
+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, index, materialized view, sequence, or TOAST table",
- RelationGetRelationName(rel))));
+ errmsg("cannot compute relpages of \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_unsuitable_relkind(rel->rd_rel->relkind,
+ RelationGetRelationName(rel))));
}
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index c39da41d2e..cd096db0a0 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -528,3 +528,48 @@ pg_nextoid(PG_FUNCTION_ARGS)
return newoid;
}
+
+/*
+ * Add an errdetail to the current error message indicating what the relkind of
+ * the relation is. This is useful when an operation is being attempted on a
+ * relation that cannot accept it.
+ */
+int
+errdetail_unsuitable_relkind(char relkind, const char *relname)
+{
+ switch (relkind)
+ {
+ case RELKIND_RELATION:
+ errdetail("\"%s\" is a plain table.", relname);
+ break;
+ case RELKIND_INDEX:
+ errdetail("\"%s\" is an index.", relname);
+ break;
+ case RELKIND_SEQUENCE:
+ errdetail("\"%s\" is a sequence.", relname);
+ break;
+ case RELKIND_TOASTVALUE:
+ errdetail("\"%s\" is a TOAST table.", relname);
+ break;
+ case RELKIND_VIEW:
+ errdetail("\"%s\" is a view.", relname);
+ break;
+ case RELKIND_MATVIEW:
+ errdetail("\"%s\" is a materialized view.", relname);
+ break;
+ case RELKIND_COMPOSITE_TYPE:
+ errdetail("\"%s\" is a composite type.", relname);
+ break;
+ case RELKIND_FOREIGN_TABLE:
+ errdetail("\"%s\" is a foreign table.", relname);
+ break;
+ case RELKIND_PARTITIONED_TABLE:
+ errdetail("\"%s\" is a partitioned table.", relname);
+ break;
+ case RELKIND_PARTITIONED_INDEX:
+ errdetail("\"%s\" is a partitioned index.", relname);
+ break;
+ }
+
+ return 0;
+}
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 77be19175a..5ecb9db90e 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -18,6 +18,7 @@
#include "access/tuptoaster.h"
#include "access/xact.h"
#include "catalog/binary_upgrade.h"
+#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/index.h"
@@ -103,8 +104,9 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
rel->rd_rel->relkind != RELKIND_MATVIEW)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table or materialized view",
- relName)));
+ errmsg("cannot create toast table for \"%s\"", relName),
+ errdetail_unsuitable_relkind(rel->rd_rel->relkind,
+ RelationGetRelationName(rel))));
/* create_toast_table does all the work */
if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0,
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 1859fb628f..f4b274e8d6 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -18,6 +18,7 @@
#include "access/htup_details.h"
#include "access/relation.h"
#include "access/table.h"
+#include "catalog/catalog.h"
#include "catalog/indexing.h"
#include "catalog/objectaddress.h"
#include "catalog/pg_description.h"
@@ -98,8 +99,10 @@ CommentObject(CommentStmt *stmt)
relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, view, materialized view, composite type, or foreign table",
- RelationGetRelationName(relation))));
+ errmsg("cannot add comment on \"%s\"",
+ RelationGetRelationName(relation)),
+ errdetail_unsuitable_relkind(relation->rd_rel->relkind,
+ RelationGetRelationName(relation))));
break;
default:
break;
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index 9db8228028..706b6d7610 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -114,8 +114,10 @@ ExecSecLabelStmt(SecLabelStmt *stmt)
relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, view, materialized view, composite type, or foreign table",
- RelationGetRelationName(relation))));
+ errmsg("cannot set security label on \"%s\"",
+ RelationGetRelationName(relation)),
+ errdetail_unsuitable_relkind(relation->rd_rel->relkind,
+ RelationGetRelationName(relation))));
break;
default:
break;
diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h
index a58d09d6c7..65d8e2b3c7 100644
--- a/src/include/catalog/catalog.h
+++ b/src/include/catalog/catalog.h
@@ -38,4 +38,6 @@ extern Oid GetNewOidWithIndex(Relation relation, Oid indexId,
extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class,
char relpersistence);
+extern int errdetail_unsuitable_relkind(char relkind, const char *relname);
+
#endif /* CATALOG_H */
--
2.17.1
--Nq2Wo0NMKNjxTN9z--
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH] Highlight that the -j option isn't passed down to vacuumdb by default.
@ 2018-12-21 10:05 jesperpedersen <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: jesperpedersen @ 2018-12-21 10:05 UTC (permalink / raw)
Author: Jesper Pedersen <[email protected]>
---
doc/src/sgml/ref/pgupgrade.sgml | 6 +++++-
src/bin/pg_upgrade/check.c | 28 ++++++++++++++++++++++------
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 7e1afaf0a5..b1da0cc7a2 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -395,7 +395,11 @@ NET STOP postgresql-&majorversion;
in parallel; a good place to start is the maximum of the number of
CPU cores and tablespaces. This option can dramatically reduce the
time to upgrade a multi-database server running on a multiprocessor
- machine.
+ machine. Note, that this option isn't passed to the
+ <application>vacuumdb</application> application by default.
+ The system environment variable <literal>VACUUMDB_OPTS</literal> can be
+ used to pass extra options to the <application>vacuumdb</application>
+ application during the script execution such as <option>--jobs</option>.
</para>
<para>
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index fc5aa7010f..5114df01b5 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -495,15 +495,31 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
ECHO_QUOTE, ECHO_QUOTE);
fprintf(script, "echo %sthis script and run:%s\n",
ECHO_QUOTE, ECHO_QUOTE);
- fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
- new_cluster.bindir, user_specification.data,
- /* Did we copy the free space files? */
- (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
- "--analyze-only" : "--analyze", ECHO_QUOTE);
+ if (user_opts.jobs <= 1 || GET_MAJOR_VERSION(new_cluster.major_version) < 905)
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
+ else
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s-j %d --all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data, user_opts.jobs,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
fprintf(script, "echo%s\n\n", ECHO_BLANK);
- fprintf(script, "\"%s/vacuumdb\" %s--all --analyze-in-stages\n",
+#ifndef WIN32
+ fprintf(script, "\"%s/vacuumdb\" %s$VACUUMDB_OPTS --all --analyze-in-stages\n",
new_cluster.bindir, user_specification.data);
+#else
+ fprintf(script, "\"%s\\vacuumdb.exe\" %s%%VACUUMDB_OPTS%% --all --analyze-in-stages\n",
+ new_cluster.bindir, user_specification.data);
+#endif
/* Did we copy the free space files? */
if (GET_MAJOR_VERSION(old_cluster.major_version) < 804)
fprintf(script, "\"%s/vacuumdb\" %s--all\n", new_cluster.bindir,
--
2.17.2
--------------D73A50015A30B21EBFE6291A--
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH] Highlight that the -j option isn't passed down to vacuumdb by default.
@ 2018-12-21 10:05 jesperpedersen <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: jesperpedersen @ 2018-12-21 10:05 UTC (permalink / raw)
Author: Jesper Pedersen <[email protected]>
---
doc/src/sgml/ref/pgupgrade.sgml | 3 ++-
src/bin/pg_upgrade/check.c | 21 ++++++++++++++++-----
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 7e1afaf0a5..937e95688d 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -395,7 +395,8 @@ NET STOP postgresql-&majorversion;
in parallel; a good place to start is the maximum of the number of
CPU cores and tablespaces. This option can dramatically reduce the
time to upgrade a multi-database server running on a multiprocessor
- machine.
+ machine. Note, that this option isn't passed to the
+ <application>vacuumdb</application> application by default.
</para>
<para>
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index fc5aa7010f..7bae43a0b1 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -495,11 +495,22 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
ECHO_QUOTE, ECHO_QUOTE);
fprintf(script, "echo %sthis script and run:%s\n",
ECHO_QUOTE, ECHO_QUOTE);
- fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
- new_cluster.bindir, user_specification.data,
- /* Did we copy the free space files? */
- (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
- "--analyze-only" : "--analyze", ECHO_QUOTE);
+ if (user_opts.jobs <= 1 || GET_MAJOR_VERSION(new_cluster.major_version) < 905)
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
+ else
+ {
+ fprintf(script, "echo %s \"%s/vacuumdb\" %s-j %d --all %s%s\n", ECHO_QUOTE,
+ new_cluster.bindir, user_specification.data, user_opts.jobs,
+ /* Did we copy the free space files? */
+ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ?
+ "--analyze-only" : "--analyze", ECHO_QUOTE);
+ }
fprintf(script, "echo%s\n\n", ECHO_BLANK);
fprintf(script, "\"%s/vacuumdb\" %s--all --analyze-in-stages\n",
--
2.17.2
--------------516817C39B28BA368D1A1A17--
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WAL Insertion Lock Improvements
@ 2023-05-08 14:48 Bharath Rupireddy <[email protected]>
2023-05-09 05:10 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Bharath Rupireddy @ 2023-05-08 14:48 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 8, 2023 at 5:57 PM Bharath Rupireddy
<[email protected]> wrote:
>
> On Mon, Apr 10, 2023 at 9:38 AM Michael Paquier <[email protected]> wrote:
>
> > -LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
> > +LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
> > [...]
> > Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
> >
> > - /* Update the lock's value */
> > - *valptr = val;
> >
> > The sensitive change is in LWLockUpdateVar(). I am not completely
> > sure to understand this removal, though. Does that influence the case
> > where there are waiters?
>
> I'll send about this in a follow-up email to not overload this
> response with too much data.
It helps the case when there are no waiters. IOW, it updates value
without waitlist lock when there are no waiters, so no extra waitlist
lock acquisition/release just to update the value. In turn, it helps
the other backend wanting to flush the WAL looking for the new updated
value of insertingAt in WaitXLogInsertionsToFinish(), now the flushing
backend can get the new value faster.
> > Another thing I was wondering about: how much does the fast-path used
> > in LWLockUpdateVar() influence the performance numbers? Am I right to
> > guess that it counts for most of the gain seen?
>
> I'll send about this in a follow-up email to not overload this
> response with too much data.
The fastpath exit in LWLockUpdateVar() doesn't seem to influence the
results much, see below results. However, it avoids waitlist lock
acquisition when there are no waiters.
test-case 1: -T5, WAL ~16 bytes
clients HEAD PATCHED with fastpath PATCHED no fast path
1 1482 1486 1457
2 1617 1620 1569
4 3174 3233 3031
8 6136 6365 5725
16 12566 12269 11685
32 24284 23621 23177
64 50135 45528 46653
128 94903 89791 89103
256 82289 152915 152835
512 62498 138838 142084
768 57083 125074 126768
1024 51308 113593 115930
2048 41084 88764 85110
4096 19939 42257 43917
> > Or could it be that
> > the removal of the spin lock in
> > LWLockConflictsWithVar()/LWLockWaitForVar() the point that has the
> > highest effect?
>
> I'll send about this in a follow-up email to not overload this
> response with too much data.
Out of 3 functions that got rid of waitlist lock
LWLockConflictsWithVar/LWLockWaitForVar, LWLockUpdateVar,
LWLockReleaseClearVar, perf reports tell that the biggest gain (for
the use-cases that I've tried) is for
LWLockConflictsWithVar/LWLockWaitForVar:
test-case 1: -T5, WAL ~16 bytes
HEAD:
+ 61.89% 0.05% postgres [.] LWLockWaitForVar
+ 43.19% 0.12% postgres [.] LWLockConflictsWithVar
+ 1.62% 0.00% postgres [.] LWLockReleaseClearVar
PATCHED:
+ 38.79% 0.11% postgres [.] LWLockWaitForVar
0.40% 0.02% postgres [.] LWLockConflictsWithVar
+ 2.80% 0.00% postgres [.] LWLockReleaseClearVar
test-case 6: -T5, WAL 4096 bytes
HEAD:
+ 29.66% 0.07% postgres [.] LWLockWaitForVar
+ 20.94% 0.08% postgres [.] LWLockConflictsWithVar
0.19% 0.03% postgres [.] LWLockUpdateVar
PATCHED:
+ 3.95% 0.08% postgres [.] LWLockWaitForVar
0.19% 0.03% postgres [.] LWLockConflictsWithVar
+ 1.73% 0.00% postgres [.] LWLockReleaseClearVar
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-05-08 14:48 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-09 05:10 ` Michael Paquier <[email protected]>
2023-05-09 06:25 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Michael Paquier @ 2023-05-09 05:10 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 08, 2023 at 08:18:04PM +0530, Bharath Rupireddy wrote:
> On Mon, May 8, 2023 at 5:57 PM Bharath Rupireddy
> <[email protected]> wrote:
>> On Mon, Apr 10, 2023 at 9:38 AM Michael Paquier <[email protected]> wrote:
>>> The sensitive change is in LWLockUpdateVar(). I am not completely
>>> sure to understand this removal, though. Does that influence the case
>>> where there are waiters?
>>
>> I'll send about this in a follow-up email to not overload this
>> response with too much data.
>
> It helps the case when there are no waiters. IOW, it updates value
> without waitlist lock when there are no waiters, so no extra waitlist
> lock acquisition/release just to update the value. In turn, it helps
> the other backend wanting to flush the WAL looking for the new updated
> value of insertingAt in WaitXLogInsertionsToFinish(), now the flushing
> backend can get the new value faster.
Sure, which is what the memory barrier given by exchange_u64
guarantees. My thoughts on this one is that I am not completely sure
to understand that we won't miss any waiters that should have been
awaken.
> The fastpath exit in LWLockUpdateVar() doesn't seem to influence the
> results much, see below results. However, it avoids waitlist lock
> acquisition when there are no waiters.
>
> test-case 1: -T5, WAL ~16 bytes
> clients HEAD PATCHED with fastpath PATCHED no fast path
> 64 50135 45528 46653
> 128 94903 89791 89103
> 256 82289 152915 152835
> 512 62498 138838 142084
> 768 57083 125074 126768
> 1024 51308 113593 115930
> 2048 41084 88764 85110
> 4096 19939 42257 43917
Considering that there could be a few percents of noise mixed into
that, that's not really surprising as the workload is highly
concurrent on inserts so the fast path won't really shine :)
Should we split this patch into two parts, as they aim at tackling two
different cases then? One for LWLockConflictsWithVar() and
LWLockReleaseClearVar() which are the straight-forward pieces
(using one pg_atomic_write_u64() in LWLockUpdateVar instead), then
a second for LWLockUpdateVar()?
Also, the fast path treatment in LWLockUpdateVar() may show some
better benefits when there are really few backends and a bunch of very
little records? Still, even that sounds a bit limited..
> Out of 3 functions that got rid of waitlist lock
> LWLockConflictsWithVar/LWLockWaitForVar, LWLockUpdateVar,
> LWLockReleaseClearVar, perf reports tell that the biggest gain (for
> the use-cases that I've tried) is for
> LWLockConflictsWithVar/LWLockWaitForVar:
>
> test-case 6: -T5, WAL 4096 bytes
> HEAD:
> + 29.66% 0.07% postgres [.] LWLockWaitForVar
> + 20.94% 0.08% postgres [.] LWLockConflictsWithVar
> 0.19% 0.03% postgres [.] LWLockUpdateVar
>
> PATCHED:
> + 3.95% 0.08% postgres [.] LWLockWaitForVar
> 0.19% 0.03% postgres [.] LWLockConflictsWithVar
> + 1.73% 0.00% postgres [.] LWLockReleaseClearVar
Indeed.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-05-08 14:48 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-09 05:10 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
@ 2023-05-09 06:25 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Michael Paquier @ 2023-05-09 06:25 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, May 09, 2023 at 02:10:20PM +0900, Michael Paquier wrote:
> Should we split this patch into two parts, as they aim at tackling two
> different cases then? One for LWLockConflictsWithVar() and
> LWLockReleaseClearVar() which are the straight-forward pieces
> (using one pg_atomic_write_u64() in LWLockUpdateVar instead), then
> a second for LWLockUpdateVar()?
I have been studying that a bit more, and I'd like to take this
suggestion back. Apologies for the noise.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v4 01/12] Change section heading to better reflect saving a result in variable(s)
@ 2023-09-24 20:49 Karl O. Pinc <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Karl O. Pinc @ 2023-09-24 20:49 UTC (permalink / raw)
The current section title of "Executing a Command with a Single-Row
Result" does not reflect what the section is really about. Other
sections make clear how to _execute_ commands, single-row result or not.
What this section is about is how to _save_ a single row of results into
variable(s).
It would be nice to talk about saving results into variables in the
section heading but I couldn't come up with anything pithy. "Saving a
Single-Row of a Command's Result" seems good enough, especially since
there's few other places to save results other than in variables.
---
doc/src/sgml/plpgsql.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index f55e901c7e..8747e84245 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1126,7 +1126,7 @@ PERFORM create_mv('cs_session_page_requests_mv', my_query);
</sect2>
<sect2 id="plpgsql-statements-sql-onerow">
- <title>Executing a Command with a Single-Row Result</title>
+ <title>Saving a Single-Row of a Command's Result</title>
<indexterm zone="plpgsql-statements-sql-onerow">
<primary>SELECT INTO</primary>
--
2.30.2
--MP_/OOXZvOwbpccKfGOtE9/SwX6
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=v4-0002-Change-section-heading-to-better-describe-referen.patch
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-09-24 20:49 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19 17:33 [PATCH v2] unsuitable relkind Alvaro Herrera <[email protected]>
2018-12-21 10:05 [PATCH] Highlight that the -j option isn't passed down to vacuumdb by default. jesperpedersen <[email protected]>
2018-12-21 10:05 [PATCH] Highlight that the -j option isn't passed down to vacuumdb by default. jesperpedersen <[email protected]>
2023-05-08 14:48 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-09 05:10 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-09 06:25 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-09-24 20:49 [PATCH v4 01/12] Change section heading to better reflect saving a result in variable(s) Karl O. Pinc <[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