public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'Masahiko Sawada' <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: RE: Design of pg_stat_subscription_workers vs pgstats
Date: Tue, 22 Feb 2022 12:22:18 +0000
Message-ID: <TYCPR01MB8373D12532FEECDAFDD5BA2BED3B9@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAD21AoAtrgBEUU08VvfzRnGS+=9rS=MEq1cTLaZ1OT51xX34dQ@mail.gmail.com>
References: <[email protected]>
<CAA4eK1Jwn5hhgHJWZ9n+wYyQEk+brfR1zkuqLjh=waji8RYWAw@mail.gmail.com>
<CAD21AoAqEt=TbUDY96Uhf9pNuh-NDR_3-CByd+xR=tFEq==1NA@mail.gmail.com>
<CAD21AoA-MVZcMA1iYwbtvTCOHx1SwG8amQUTT8j2ssh2zuc6OQ@mail.gmail.com>
<[email protected]>
<CAD21AoDTO3bJvrc39Tv3VCeyUzZpimXnJuRhHQhKyjOWorHeOA@mail.gmail.com>
<[email protected]>
<CAD21AoA_r+Wc-zJZdy4Lii3oOvvdGcLwrY=2Y4y63HueYSd6Eg@mail.gmail.com>
<[email protected]>
<CAA4eK1+j00Pc+p=AfJLXh-a+OJ-a0UD4adMr2tRkQvKG_LggHA@mail.gmail.com>
<[email protected]>
<CAA4eK1J3dhfMRNQ6Q585nFr1vmXaTXVW9qv4hmUxLwDwJp-9RQ@mail.gmail.com>
<CAD21AoAtrgBEUU08VvfzRnGS+=9rS=MEq1cTLaZ1OT51xX34dQ@mail.gmail.com>
On Tuesday, February 22, 2022 2:45 PM Masahiko Sawada <[email protected]> wrote:
> I've attached a patch that changes pg_stat_subscription_workers view.
> It removes non-cumulative values such as error details such as error-XID and
> the error message from the view, and consequently the view now has only
> cumulative statistics counters: apply_error_count and sync_error_count. Since
> the new view name is under discussion I temporarily chose
> pg_stat_subscription_activity.
Hi, thank you for sharing the patch.
Few minor comments for v1.
(1) commit message's typo
This commits changes the view so that it stores only statistics
counters: apply_error_count and sync_error_count.
"This commits" -> "This commit"
(2) minor improvement suggestion for the commit message
I suggest that we touch the commit id 8d74fc9
that introduces the pg_stat_subscription_workers
in the commit message, for better traceability. Below is an example.
From:
As the result of the discussion, we've concluded that the stats
collector is not an appropriate place to store the error information of
subscription workers.
To:
As the result of the discussion about the view introduced by 8d74fc9,...
(3) doc/src/sgml/logical-replication.sgml
Kindly refer to commit id 85c61ba for the detail.
You forgot "the" in the below sentence.
@@ -346,8 +346,6 @@
<para>
A conflict will produce an error and will stop the replication; it must be
resolved manually by the user. Details about the conflict can be found in
- <link linkend="monitoring-pg-stat-subscription-workers">
- <structname>pg_stat_subscription_workers</structname></link> and the
subscriber's server log.
</para>
From:
subscriber's server log.
to:
the subscriber's server log.
(4) doc/src/sgml/monitoring.sgml
<row>
<entry role="catalog_table_entry"><para role="column_definition">
- <structfield>last_error_time</structfield> <type>timestamp with time zone</type>
+ <structfield>sync_error_count</structfield> <type>uint8</type>
</para>
<para>
- Last time at which this error occurred
+ Number of times the error occurred during the initial data copy
</para></entry>
I supposed it might be better to use "initial data sync"
or "initial data synchronization", rather than "initial data copy".
(5) src/test/subscription/t/026_worker_stats.pl
+# Truncate test_tab1 so that table sync can continue.
+$node_subscriber->safe_psql('postgres', "TRUNCATE test_tab1;");
The second truncate is for apply, isn't it? Therefore, kindly change
From:
Truncate test_tab1 so that table sync can continue.
To:
Truncate test_tab1 so that apply can continue.
(6) src/test/subscription/t/026_worker_stats.pl
+# Insert more data to test_tab1 on the subscriber and then on the publisher, raising an
+# error on the subscriber due to violation of the unique constraint on test_tab1.
+$node_subscriber->safe_psql('postgres', "INSERT INTO test_tab1 VALUES (2)");
Did we need this insert ?
If you want to indicate the apply is working okay after the error of table sync is solved,
waiting for the max value in the test_tab1 becoming 2 on the subscriber by polling query
would work. But, I was not sure if this is essentially necessary for the testing purpose.
Best Regards,
Takamichi Osumi
view thread (40+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Design of pg_stat_subscription_workers vs pgstats
In-Reply-To: <TYCPR01MB8373D12532FEECDAFDD5BA2BED3B9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox