On Tue, 9 Feb 2021 at 08:12, Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Tue, Feb 9, 2021 at 12:06 AM Matthias van de Meent > <boekewurm+postg...@gmail.com> wrote: > > With [0] we got COPY progress reporting. Before the column names of > > this newly added view are effectively set in stone with the release of > > pg14, I propose the following set of relatively small patches. These > > are v2, because it is a patchset that is based on a set of patches > > that I previously posted in [0]. > > Thanks for working on the patches. Here are some comments: > > 0001 - +1 to add tuples_excluded and the patch LGTM. > > 0002 - Yes, the tuples_processed or tuples_excluded makes more sense > to me than lines_processed and lines_excluded. The patch LGTM. > > 0003 - Instead of just adding the progress reporting to "See also" > sections in the footer of the respective pages analyze, cluster and > others, it would be nice if we have a mention of it in the description > as pg_basebackup has something like below: > <para> > Whenever <application>pg_basebackup</application> is taking a base > backup, the server's <structname>pg_stat_progress_basebackup</structname> > view will report the progress of the backup. > See <xref linkend="basebackup-progress-reporting"/> for details.
Added > 0004 - > 1) How about PROGRESS_COPY_COMMAND_TYPE instead of > PROGRESS_COPY_COMMAND? The names looks bit confusing with the existing > PROGRESS_COMMAND_COPY. The current name is consistent with the naming of the other command-reporting progress views; CREATEIDX and CLUSTER both use the *_COMMAND as this column indexes' internal name. > 0005 - > 1) How about > + or <literal>CALLBACK</literal> (used in the table > synchronization background > + worker). > instead of > + or <literal>CALLBACK</literal> (used in the tablesync background > + worker). > Because "table synchronization" is being used in logical-replication.sgml. Fixed > 2) I think cstate->copy_src = COPY_CALLBACK is assigned after the > switch case added in copyfrom.c > if (data_source_cb) > { > cstate->copy_src = COPY_CALLBACK; > cstate->data_source_cb = data_source_cb; > } Yes, I noticed this too while working on the patchset, but apparently didn't act on this... Fixed in attachted version. > Also, you can add this to the current commitfest. See https://commitfest.postgresql.org/32/2977/ On Tue, 9 Feb 2021 at 12:53, Josef Šimánek <josef.sima...@gmail.com> wrote: > > OK, would you mind to integrate my regression test initial patch as > well in v3 or should I submit it later in a separate way? Attached, with minor fixes With regards, Matthias van de Meent
From 720f58890c2910bba7829c380ccb5a022596a6c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= <josef.sima...@gmail.com> Date: Tue, 9 Feb 2021 13:06:45 +0100 Subject: [PATCH v3 6/6] Add progress reporting regression tests This tests some basic features of copy progress reporting, as this is possible to implement in one session using triggers. Other views may follow. --- src/test/regress/expected/progress.out | 47 ++++++++++++++++++++++++++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/progress.sql | 43 +++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/test/regress/expected/progress.out create mode 100644 src/test/regress/sql/progress.sql diff --git a/src/test/regress/expected/progress.out b/src/test/regress/expected/progress.out new file mode 100644 index 0000000000..8feed9df42 --- /dev/null +++ b/src/test/regress/expected/progress.out @@ -0,0 +1,47 @@ +-- setup for COPY progress testing +CREATE TEMP TABLE test_progress_with_trigger ( + a int, + b text +) ; +CREATE function notice_after_progress_reporting() RETURNS trigger AS +$$ +DECLARE report record; +BEGIN + SELECT INTO report * FROM pg_stat_progress_copy report WHERE pid = pg_backend_pid(); + raise info 'progress datname: %', report.datname; + raise info 'progress command: %', report.command; + raise info 'progress io_target: %', report.io_target; + raise info 'progress bytes_processed: %', report.bytes_processed; + raise info 'progress bytes_total: %', report.bytes_total; + raise info 'progress tuples_processed: %', report.tuples_processed; + raise info 'progress tuples_excluded: %', report.tuples_excluded; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; +CREATE TRIGGER check_after_progress_reporting + AFTER INSERT ON test_progress_with_trigger + FOR EACH ROW + EXECUTE FUNCTION notice_after_progress_reporting(); +-- simple COPY from STDIN +COPY test_progress_with_trigger (a, b) FROM STDIN; +INFO: progress datname: regression +INFO: progress command: COPY FROM +INFO: progress io_target: STDIO +INFO: progress bytes_processed: 9 +INFO: progress bytes_total: 0 +INFO: progress tuples_processed: 1 +INFO: progress tuples_excluded: 0 +-- COPY from STDIN with WHERE skipping lines +COPY test_progress_with_trigger (a, b) FROM STDIN WHERE a > 1; +INFO: progress datname: regression +INFO: progress command: COPY FROM +INFO: progress io_target: STDIO +INFO: progress bytes_processed: 18 +INFO: progress bytes_total: 0 +INFO: progress tuples_processed: 1 +INFO: progress tuples_excluded: 1 +-- cleanup +DROP TRIGGER check_after_progress_reporting ON test_progress_with_trigger; +DROP FUNCTION notice_after_progress_reporting; +DROP TABLE test_progress_with_trigger; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 12bb67e491..e445ad24f9 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -97,7 +97,7 @@ test: publication subscription # ---------- # Another group of parallel tests # ---------- -test: select_views portals_p2 foreign_key cluster dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass +test: select_views portals_p2 foreign_key cluster dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass progress # ---------- # Another group of parallel tests (JSON related) diff --git a/src/test/regress/sql/progress.sql b/src/test/regress/sql/progress.sql new file mode 100644 index 0000000000..9b25bc027d --- /dev/null +++ b/src/test/regress/sql/progress.sql @@ -0,0 +1,43 @@ +-- setup for COPY progress testing +CREATE TEMP TABLE test_progress_with_trigger ( + a int, + b text +) ; + +CREATE function notice_after_progress_reporting() RETURNS trigger AS +$$ +DECLARE report record; +BEGIN + SELECT INTO report * FROM pg_stat_progress_copy report WHERE pid = pg_backend_pid(); + raise info 'progress datname: %', report.datname; + raise info 'progress command: %', report.command; + raise info 'progress io_target: %', report.io_target; + raise info 'progress bytes_processed: %', report.bytes_processed; + raise info 'progress bytes_total: %', report.bytes_total; + raise info 'progress tuples_processed: %', report.tuples_processed; + raise info 'progress tuples_excluded: %', report.tuples_excluded; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER check_after_progress_reporting + AFTER INSERT ON test_progress_with_trigger + FOR EACH ROW + EXECUTE FUNCTION notice_after_progress_reporting(); + +-- simple COPY from STDIN +COPY test_progress_with_trigger (a, b) FROM STDIN; +1 test_1 +\. + +-- COPY from STDIN with WHERE skipping lines +COPY test_progress_with_trigger (a, b) FROM STDIN WHERE a > 1; +1 test_1 +2 test_2 +\. + +-- cleanup +DROP TRIGGER check_after_progress_reporting ON test_progress_with_trigger; +DROP FUNCTION notice_after_progress_reporting; +DROP TABLE test_progress_with_trigger; -- 2.20.1
From f7d761f6774753d4914d0dbc80effbb1ab09b58e Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekew...@gmail.com> Date: Mon, 8 Feb 2021 17:36:00 +0100 Subject: [PATCH v3 4/6] Add a command column to the copy progress view This allows filtering on COPY FROM / COPY TO for progress reporting, and makes it possible to determine the further meaning of the columns involved. --- doc/src/sgml/monitoring.sgml | 10 ++++++++++ src/backend/catalog/system_views.sql | 3 +++ src/backend/commands/copyfrom.c | 1 + src/backend/commands/copyto.c | 1 + src/include/commands/progress.h | 5 +++++ src/test/regress/expected/rules.out | 5 +++++ 6 files changed, 25 insertions(+) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 940e9dcee4..ca84b53896 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6544,6 +6544,16 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>command</structfield> <type>text</type> + </para> + <para> + The command that is running: <literal>COPY FROM</literal>, or + <literal>COPY TO</literal>. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>bytes_processed</structfield> <type>bigint</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index e7e227792c..1082b7d253 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1129,6 +1129,9 @@ CREATE VIEW pg_stat_progress_copy AS SELECT S.pid AS pid, S.datid AS datid, D.datname AS datname, S.relid AS relid, + CASE S.param5 WHEN 1 THEN 'COPY FROM' + WHEN 2 THEN 'COPY TO' + END AS command, S.param1 AS bytes_processed, S.param2 AS bytes_total, S.param3 AS tuples_processed, diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index fb3c7e2c0c..ce343dbf80 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -1428,6 +1428,7 @@ BeginCopyFrom(ParseState *pstate, /* initialize progress */ pgstat_progress_start_command(PROGRESS_COMMAND_COPY, cstate->rel ? RelationGetRelid(cstate->rel) : InvalidOid); + pgstat_progress_update_param(PROGRESS_COPY_COMMAND, PROGRESS_COPY_COMMAND_FROM); cstate->bytes_processed = 0; /* We keep those variables in cstate. */ diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 9ffe7a6ee3..534c091c75 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -772,6 +772,7 @@ BeginCopyTo(ParseState *pstate, /* initialize progress */ pgstat_progress_start_command(PROGRESS_COMMAND_COPY, cstate->rel ? RelationGetRelid(cstate->rel) : InvalidOid); + pgstat_progress_update_param(PROGRESS_COPY_COMMAND, PROGRESS_COPY_COMMAND_TO); cstate->bytes_processed = 0; MemoryContextSwitchTo(oldcontext); diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index 8b2b188bd5..1c30d09abb 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -138,5 +138,10 @@ #define PROGRESS_COPY_BYTES_TOTAL 1 #define PROGRESS_COPY_TUPLES_PROCESSED 2 #define PROGRESS_COPY_TUPLES_EXCLUDED 3 +#define PROGRESS_COPY_COMMAND 4 + +/* Commands of PROGRESS_COPY_COMMAND */ +#define PROGRESS_COPY_COMMAND_FROM 1 +#define PROGRESS_COPY_COMMAND_TO 2 #endif diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 970f6909c2..63b5e33083 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1948,6 +1948,11 @@ pg_stat_progress_copy| SELECT s.pid, s.datid, d.datname, s.relid, + CASE s.param5 + WHEN 1 THEN 'COPY FROM'::text + WHEN 2 THEN 'COPY TO'::text + ELSE NULL::text + END AS command, s.param1 AS bytes_processed, s.param2 AS bytes_total, s.param3 AS tuples_processed, -- 2.20.1
From 1aaf82200c79aaec10a81f669fa49b801ff8b4ed Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekew...@gmail.com> Date: Fri, 5 Feb 2021 23:11:50 +0100 Subject: [PATCH v3 3/6] Add backlinks to progress reporting documentation Previously, for most progress-reported features, the only place the feature was mentioned is in the progress reporting document itself. This makes the progress reporting more discoverable from the reported commands. --- doc/src/sgml/ref/analyze.sgml | 7 +++++++ doc/src/sgml/ref/cluster.sgml | 6 ++++++ doc/src/sgml/ref/copy.sgml | 14 ++++++++++++++ doc/src/sgml/ref/create_index.sgml | 8 ++++++++ doc/src/sgml/ref/pg_basebackup.sgml | 1 + doc/src/sgml/ref/reindex.sgml | 7 +++++++ doc/src/sgml/ref/vacuum.sgml | 11 +++++++++++ 7 files changed, 54 insertions(+) diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml index 7d816c87c6..9db9070b62 100644 --- a/doc/src/sgml/ref/analyze.sgml +++ b/doc/src/sgml/ref/analyze.sgml @@ -273,6 +273,12 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea will not record new statistics for that table. Any existing statistics will be retained. </para> + + <para> + Each backend running the <command>ANALYZE</command> command will report their + progress to the <structname>pg_stat_progress_analyze</structname> view. + See <xref linkend="analyze-progress-reporting"/> for details. + </para> </refsect1> <refsect1> @@ -291,6 +297,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea <member><xref linkend="app-vacuumdb"/></member> <member><xref linkend="runtime-config-resource-vacuum-cost"/></member> <member><xref linkend="autovacuum"/></member> + <member><xref linkend="analyze-progress-reporting"/></member> </simplelist> </refsect1> </refentry> diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 5dd21a0189..5c2270f71b 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -192,6 +192,11 @@ CLUSTER [VERBOSE] are periodically reclustered. </para> + <para> + Each backend running the <command>CLUSTER</command> command will report their + progress to the <structname>pg_stat_progress_cluster</structname> view. + See <xref linkend="cluster-progress-reporting"/> for details. + </para> </refsect1> <refsect1> @@ -242,6 +247,7 @@ CLUSTER <replaceable class="parameter">index_name</replaceable> ON <replaceable <simplelist type="inline"> <member><xref linkend="app-clusterdb"/></member> + <member><xref linkend="cluster-progress-reporting"/></member> </simplelist> </refsect1> </refentry> diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml index 0fca6583af..af3ce72561 100644 --- a/doc/src/sgml/ref/copy.sgml +++ b/doc/src/sgml/ref/copy.sgml @@ -82,6 +82,12 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable specified, data is transmitted via the connection between the client and the server. </para> + + <para> + Each backend running the <command>COPY</command> command will report their + progress to the <structname>pg_stat_progress_copy</structname> view. + See <xref linkend="copy-progress-reporting"/> for details. + </para> </refsect1> <refsect1> @@ -1052,4 +1058,12 @@ COPY [ BINARY ] <replaceable class="parameter">table_name</replaceable> [ WITH NULL AS '<replaceable class="parameter">null_string</replaceable>' ] </synopsis></para> </refsect1> + + <refsect1> + <title>See Also</title> + + <simplelist type="inline"> + <member><xref linkend="copy-progress-reporting"/></member> + </simplelist> + </refsect1> </refentry> diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml index a5271a9f8f..278058f500 100644 --- a/doc/src/sgml/ref/create_index.sgml +++ b/doc/src/sgml/ref/create_index.sgml @@ -865,6 +865,13 @@ Indexes: will interpret it as <literal>USING gist</literal>, to simplify conversion of old databases to GiST. </para> + + <para> + Each backend running the <command>CREATE INDEX</command> command will + report their progress to the + <structname>pg_stat_progress_create_index</structname> view. + See <xref linkend="create-index-progress-reporting"/> for details. + </para> </refsect1> <refsect1> @@ -978,6 +985,7 @@ CREATE INDEX CONCURRENTLY sales_quantity_index ON sales_table (quantity); <member><xref linkend="sql-alterindex"/></member> <member><xref linkend="sql-dropindex"/></member> <member><xref linkend="sql-reindex"/></member> + <member><xref linkend="create-index-progress-reporting"/></member> </simplelist> </refsect1> </refentry> diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 5754ad5aa6..14cc88a852 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -904,6 +904,7 @@ PostgreSQL documentation <simplelist type="inline"> <member><xref linkend="app-pgdump"/></member> + <member><xref linkend="basebackup-progress-reporting"/></member> </simplelist> </refsect1> diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 07795b5737..f69f5db403 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -493,6 +493,12 @@ Indexes: is reindexed concurrently, those indexes will be skipped. (It is possible to reindex such indexes without the <command>CONCURRENTLY</command> option.) </para> + + <para> + Each backend running the <command>REINDEX</command> command will report + their progress to the <structname>pg_stat_progress_create_index</structname> + view. See <xref linkend="create-index-progress-reporting"/> for details. + </para> </refsect2> </refsect1> @@ -551,6 +557,7 @@ REINDEX TABLE CONCURRENTLY my_broken_table; <member><xref linkend="sql-createindex"/></member> <member><xref linkend="sql-dropindex"/></member> <member><xref linkend="app-reindexdb"/></member> + <member><xref linkend="create-index-progress-reporting"/></member> </simplelist> </refsect1> </refentry> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 21ab57d880..f087087816 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -378,6 +378,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet information about automatic and manual vacuuming, see <xref linkend="routine-vacuuming"/>. </para> + <para> + Each backend running the <command>VACUUM</command> command without the + <literal>FULL</literal> option will report their progress in the + <structname>pg_stat_progress_vacuum</structname> view. Backends running + <command>VACUUM</command> with the <literal>FULL</literal> option report + progress in the <structname>pg_stat_progress_cluster</structname> instead. + See <xref linkend="vacuum-progress-reporting"/> and + <xref linkend="cluster-progress-reporting"/> for details. + </para> </refsect1> <refsect1> @@ -407,6 +416,8 @@ VACUUM (VERBOSE, ANALYZE) onek; <member><xref linkend="app-vacuumdb"/></member> <member><xref linkend="runtime-config-resource-vacuum-cost"/></member> <member><xref linkend="autovacuum"/></member> + <member><xref linkend="vacuum-progress-reporting"/></member> + <member><xref linkend="cluster-progress-reporting"/></member> </simplelist> </refsect1> </refentry> -- 2.20.1
From d6723f2cfbe98e9837948336420f283312c91b51 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekew...@gmail.com> Date: Thu, 7 Jan 2021 16:51:30 +0100 Subject: [PATCH v3 2/6] Rename lines* to tuples* in COPY progress reporting view This pulls the names of the columns in line with the other progress reporting views and the comments surrounding the code that updates the columns. It also makes it clear to the user that for the foreign data consumed or produced in the COPY command, the line count need not be related to these columns. --- doc/src/sgml/monitoring.sgml | 8 ++++---- src/backend/catalog/system_views.sql | 4 ++-- src/backend/commands/copyfrom.c | 4 ++-- src/backend/commands/copyto.c | 4 ++-- src/include/commands/progress.h | 4 ++-- src/test/regress/expected/rules.out | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index dc998bc5f7..940e9dcee4 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6565,19 +6565,19 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, <row> <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>lines_processed</structfield> <type>bigint</type> + <structfield>tuples_processed</structfield> <type>bigint</type> </para> <para> - Number of lines already processed by <command>COPY</command> command. + Number of tuples already processed by <command>COPY</command> command. </para></entry> </row> <row> <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>lines_excluded</structfield> <type>bigint</type> + <structfield>tuples_excluded</structfield> <type>bigint</type> </para> <para> - Number of lines not processed because they were excluded by the + Number of tuples not processed because they were excluded by the <command>WHERE</command> clause of the <command>COPY</command> command. </para></entry> </row> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index a9bd983419..e7e227792c 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1131,8 +1131,8 @@ CREATE VIEW pg_stat_progress_copy AS S.relid AS relid, S.param1 AS bytes_processed, S.param2 AS bytes_total, - S.param3 AS lines_processed, - S.param4 AS lines_excluded + S.param3 AS tuples_processed, + S.param4 AS tuples_excluded FROM pg_stat_get_progress_info('COPY') AS S LEFT JOIN pg_database D ON S.datid = D.oid; diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 86e6a422e2..fb3c7e2c0c 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -871,7 +871,7 @@ CopyFrom(CopyFromState cstate) if (!ExecQual(cstate->qualexpr, econtext)) { /* Report that this tuple was filtered out by the WHERE clause */ - pgstat_progress_update_param(PROGRESS_COPY_LINES_EXCLUDED, ++excluded); + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_EXCLUDED, ++excluded); continue; } } @@ -1111,7 +1111,7 @@ CopyFrom(CopyFromState cstate) * for counting tuples inserted by an INSERT command. Update * progress of the COPY command as well. */ - pgstat_progress_update_param(PROGRESS_COPY_LINES_PROCESSED, ++processed); + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, ++processed); } } diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index e04ec1e331..9ffe7a6ee3 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -954,7 +954,7 @@ CopyTo(CopyToState cstate) CopyOneRowTo(cstate, slot); /* Increment amount of processed tuples and update the progress */ - pgstat_progress_update_param(PROGRESS_COPY_LINES_PROCESSED, ++processed); + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, ++processed); } ExecDropSingleTupleTableSlot(slot); @@ -1321,7 +1321,7 @@ copy_dest_receive(TupleTableSlot *slot, DestReceiver *self) CopyOneRowTo(cstate, slot); /* Increment amount of processed tuples and update the progress */ - pgstat_progress_update_param(PROGRESS_COPY_LINES_PROCESSED, ++myState->processed); + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, ++myState->processed); return true; } diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index e6f6545033..8b2b188bd5 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -136,7 +136,7 @@ /* Commands of PROGRESS_COPY */ #define PROGRESS_COPY_BYTES_PROCESSED 0 #define PROGRESS_COPY_BYTES_TOTAL 1 -#define PROGRESS_COPY_LINES_PROCESSED 2 -#define PROGRESS_COPY_LINES_EXCLUDED 3 +#define PROGRESS_COPY_TUPLES_PROCESSED 2 +#define PROGRESS_COPY_TUPLES_EXCLUDED 3 #endif diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 25a863627e..970f6909c2 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1950,8 +1950,8 @@ pg_stat_progress_copy| SELECT s.pid, s.relid, s.param1 AS bytes_processed, s.param2 AS bytes_total, - s.param3 AS lines_processed, - s.param4 AS lines_excluded + s.param3 AS tuples_processed, + s.param4 AS tuples_excluded FROM (pg_stat_get_progress_info('COPY'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20) LEFT JOIN pg_database d ON ((s.datid = d.oid))); pg_stat_progress_create_index| SELECT s.pid, -- 2.20.1
From 35e9815dec8e1770d380d6b2446b41010b1007a0 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekew...@gmail.com> Date: Mon, 8 Feb 2021 18:13:46 +0100 Subject: [PATCH v3 5/6] Add a io_target column to the copy progress view This allows filtering on IO target for progress reporting, and allows for further filtering of COPY commands. Additionally, this allows for identification of logical replication's initial table synchronization background workers at the subscriber side through io_target = CALLBACK, as it is the only current supplier of a callback datasource. --- doc/src/sgml/monitoring.sgml | 13 +++++++++++++ src/backend/catalog/system_views.sql | 5 +++++ src/backend/commands/copyfrom.c | 22 ++++++++++++++++++++++ src/backend/commands/copyto.c | 26 +++++++++++++++++++++++++- src/include/commands/progress.h | 7 +++++++ src/test/regress/expected/rules.out | 7 +++++++ 6 files changed, 79 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index ca84b53896..3c39c82f1a 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6554,6 +6554,19 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>io_target</structfield> <type>text</type> + </para> + <para> + The io target that the data is read from or written to: + <literal>FILE</literal>, <literal>PROGRAM</literal>, + <literal>STDIO</literal> (for COPY FROM STDIN and COPY TO STDOUT), + or <literal>CALLBACK</literal> (used in the table synchronization + background worker). + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>bytes_processed</structfield> <type>bigint</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 1082b7d253..6a3ac47b85 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1132,6 +1132,11 @@ CREATE VIEW pg_stat_progress_copy AS CASE S.param5 WHEN 1 THEN 'COPY FROM' WHEN 2 THEN 'COPY TO' END AS command, + CASE S.param6 WHEN 1 THEN 'FILE' + WHEN 2 THEN 'PROGRAM' + WHEN 3 THEN 'STDIO' + WHEN 4 THEN 'CALLBACK' + END AS io_target, S.param1 AS bytes_processed, S.param2 AS bytes_total, S.param3 AS tuples_processed, diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index ce343dbf80..bf952fa293 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -1429,6 +1429,7 @@ BeginCopyFrom(ParseState *pstate, pgstat_progress_start_command(PROGRESS_COMMAND_COPY, cstate->rel ? RelationGetRelid(cstate->rel) : InvalidOid); pgstat_progress_update_param(PROGRESS_COPY_COMMAND, PROGRESS_COPY_COMMAND_FROM); + cstate->bytes_processed = 0; /* We keep those variables in cstate. */ @@ -1506,6 +1507,27 @@ BeginCopyFrom(ParseState *pstate, ReceiveCopyBinaryHeader(cstate); } + { + int64 io_target; + switch (cstate->copy_src) + { + case COPY_FILE: + if (is_program) + io_target = PROGRESS_COPY_IO_TARGET_PROGRAM; + else + io_target = PROGRESS_COPY_IO_TARGET_FILE; + break; + case COPY_OLD_FE: + case COPY_NEW_FE: + io_target = PROGRESS_COPY_IO_TARGET_STDIO; + break; + case COPY_CALLBACK: + io_target = PROGRESS_COPY_IO_TARGET_CALLBACK; + break; + } + pgstat_progress_update_param(PROGRESS_COPY_IO_TARGET, io_target); + } + /* create workspace for CopyReadAttributes results */ if (!cstate->opts.binary) { diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 534c091c75..42c4a828df 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -772,7 +772,31 @@ BeginCopyTo(ParseState *pstate, /* initialize progress */ pgstat_progress_start_command(PROGRESS_COMMAND_COPY, cstate->rel ? RelationGetRelid(cstate->rel) : InvalidOid); - pgstat_progress_update_param(PROGRESS_COPY_COMMAND, PROGRESS_COPY_COMMAND_TO); + { + const int progress_index[] = { + PROGRESS_COPY_COMMAND, + PROGRESS_COPY_IO_TARGET + }; + int64 progress_vals[] = { + PROGRESS_COPY_COMMAND_TO, + 0 + }; + switch (cstate->copy_dest) + { + case COPY_FILE: + if (is_program) + progress_vals[1] = PROGRESS_COPY_IO_TARGET_PROGRAM; + else + progress_vals[1] = PROGRESS_COPY_IO_TARGET_FILE; + break; + case COPY_OLD_FE: + case COPY_NEW_FE: + progress_vals[1] = PROGRESS_COPY_IO_TARGET_STDIO; + break; + } + pgstat_progress_update_multi_param(2, progress_index, progress_vals); + } + cstate->bytes_processed = 0; MemoryContextSwitchTo(oldcontext); diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index 1c30d09abb..e003217554 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -139,9 +139,16 @@ #define PROGRESS_COPY_TUPLES_PROCESSED 2 #define PROGRESS_COPY_TUPLES_EXCLUDED 3 #define PROGRESS_COPY_COMMAND 4 +#define PROGRESS_COPY_IO_TARGET 5 /* Commands of PROGRESS_COPY_COMMAND */ #define PROGRESS_COPY_COMMAND_FROM 1 #define PROGRESS_COPY_COMMAND_TO 2 +/* Types of PROGRESS_COPY_INOUT_TYPE */ +#define PROGRESS_COPY_IO_TARGET_FILE 1 +#define PROGRESS_COPY_IO_TARGET_PROGRAM 2 +#define PROGRESS_COPY_IO_TARGET_STDIO 3 +#define PROGRESS_COPY_IO_TARGET_CALLBACK 4 + #endif diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 63b5e33083..0698c71d23 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1953,6 +1953,13 @@ pg_stat_progress_copy| SELECT s.pid, WHEN 2 THEN 'COPY TO'::text ELSE NULL::text END AS command, + CASE s.param6 + WHEN 1 THEN 'FILE'::text + WHEN 2 THEN 'PROGRAM'::text + WHEN 3 THEN 'STDIO'::text + WHEN 4 THEN 'CALLBACK'::text + ELSE NULL::text + END AS io_target, s.param1 AS bytes_processed, s.param2 AS bytes_total, s.param3 AS tuples_processed, -- 2.20.1
From 177fabe331cbf21b695c9d8f7bab513d14e8f727 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekewurm+postg...@gmail.com> Date: Thu, 7 Jan 2021 16:39:57 +0100 Subject: [PATCH v3 1/6] Add progress reporting for excluded rows. COPY ... FROM ... WHERE (condition) will exclude rows that do not match the condition, which we now register as explicitly being excluded from insertion. --- doc/src/sgml/monitoring.sgml | 10 ++++++++++ src/backend/catalog/system_views.sql | 3 ++- src/backend/commands/copyfrom.c | 5 +++++ src/include/commands/progress.h | 1 + src/test/regress/expected/rules.out | 3 ++- 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index c602ee4427..dc998bc5f7 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6571,6 +6571,16 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, Number of lines already processed by <command>COPY</command> command. </para></entry> </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>lines_excluded</structfield> <type>bigint</type> + </para> + <para> + Number of lines not processed because they were excluded by the + <command>WHERE</command> clause of the <command>COPY</command> command. + </para></entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index fa58afd9d7..a9bd983419 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1131,7 +1131,8 @@ CREATE VIEW pg_stat_progress_copy AS S.relid AS relid, S.param1 AS bytes_processed, S.param2 AS bytes_total, - S.param3 AS lines_processed + S.param3 AS lines_processed, + S.param4 AS lines_excluded FROM pg_stat_get_progress_info('COPY') AS S LEFT JOIN pg_database D ON S.datid = D.oid; diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index c39cc736ed..86e6a422e2 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -540,6 +540,7 @@ CopyFrom(CopyFromState cstate) CopyInsertMethod insertMethod; CopyMultiInsertInfo multiInsertInfo = {0}; /* pacify compiler */ uint64 processed = 0; + uint64 excluded = 0; bool has_before_insert_row_trig; bool has_instead_insert_row_trig; bool leafpart_use_multi_insert = false; @@ -868,7 +869,11 @@ CopyFrom(CopyFromState cstate) econtext->ecxt_scantuple = myslot; /* Skip items that don't match COPY's WHERE clause */ if (!ExecQual(cstate->qualexpr, econtext)) + { + /* Report that this tuple was filtered out by the WHERE clause */ + pgstat_progress_update_param(PROGRESS_COPY_LINES_EXCLUDED, ++excluded); continue; + } } /* Determine the partition to insert the tuple into */ diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index 95ec5d02e9..e6f6545033 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -137,5 +137,6 @@ #define PROGRESS_COPY_BYTES_PROCESSED 0 #define PROGRESS_COPY_BYTES_TOTAL 1 #define PROGRESS_COPY_LINES_PROCESSED 2 +#define PROGRESS_COPY_LINES_EXCLUDED 3 #endif diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 6173473de9..25a863627e 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1950,7 +1950,8 @@ pg_stat_progress_copy| SELECT s.pid, s.relid, s.param1 AS bytes_processed, s.param2 AS bytes_total, - s.param3 AS lines_processed + s.param3 AS lines_processed, + s.param4 AS lines_excluded FROM (pg_stat_get_progress_info('COPY'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20) LEFT JOIN pg_database d ON ((s.datid = d.oid))); pg_stat_progress_create_index| SELECT s.pid, -- 2.20.1