Re: locking [user] catalog tables vs 2pc vs logical rep - Mailing list pgsql-hackers

On Fri, Jun 11, 2021 at 6:57 AM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> On Thursday, June 10, 2021 1:30 PM I wrote:
> > On Thursday, June 10, 2021 1:14 PM vignesh C <vignesh21@gmail.com>
> > > On Wed, Jun 9, 2021 at 12:03 PM osumi.takamichi@fujitsu.com
> > > <osumi.takamichi@fujitsu.com> wrote:
> > > >
> > > > On Wednesday, June 9, 2021 12:06 PM Amit Kapila
> > > <amit.kapila16@gmail.com> wrote:
> > > > > On Tue, Jun 8, 2021 at 6:24 PM vignesh C <vignesh21@gmail.com>
> > wrote:
> > > > > >
> > > > > > Thanks for the updated patch.
> > > > > >
> > > > > > I have few comments:
> > > > > > 1) Should we list the actual system tables like
> > > > > > pg_class,pg_trigger, etc instead of any other catalog table?
> > > > > > User has issued an explicit LOCK on pg_class (or any other
> > > > > > catalog
> > > > > > table)
> > > > > >
> > > > >
> > > > > I think the way it is mentioned is okay. We don't need to specify
> > > > > other catalog tables.
> > > > Okay.
> > > >
> > > >
> > > > > > 2) Here This means deadlock, after this we mention deadlock
> > > > > > again for each of the examples, we can remove it if redundant.
> > > > > > This can happen in the following ways:
> > > > I think this sentence works to notify that commands described below
> > > > are major scenarios naturally, to the readers. Then, I don't want to
> > > > remove
> > > it.
> > > >
> > > > If you somehow feel that the descriptions are redundant, how about
> > > > unifying all listitems as nouns. like below ?
> > > >
> > > > * An explicit <command>LOCK</command> on
> > > > <structname>pg_class</structname> (or any other catalog table) in a
> > > > transaction
> > > > * Reordering <structname>pg_class</structname> by
> > > > <command>CLUSTER</command> command in a transaction
> > > > * Executing <command>TRUNCATE</command> on
> > user_catalog_table
> > > >
> > >
> > > This looks good to me. Keep the 2PC documentation patch also on the
> > > same lines.
> > Yeah, of course. Thanks for your confirmation.
> Hi, attached the updated patch-set.
>
> I've conducted some updates.
>
> (1) Added commit messages for all patches
> (2) Sorted out the descriptions of listitem to make them look uniform
> (3) Removed PG11-specific patch and unified the patch from PG11 to PG13,
> which will keep the documents cleanliness for future back-patching, if any.
>
> (4) Removed unnecessary space after 'id'
>
> In v04, there was an unneeded space like below. Fixed.
> In the same logicaldecoding.sgml doc, there is no space after 'id' for sec2.
>
> +   <sect2 id ="logicaldecoding-synchronous-caveats">
> +    <title>Caveats</title>
>
> (5) Fixed the reference accurately by replacing link tag with xref tag.
>
> In v04, I let the reference be inaccurate, because the linkend points to the caveats
> but the link word was "Synchronous Replication Support for Logical Decoding".
>
> +       [user] catalog tables exclusively. To avoid this users must refrain from
> +       having locks on catalog tables (e.g. explicit <command>LOCK</command> command)
> +       in such transactions.
> +       (See <link linkend="logicaldecoding-synchronous-caveats">Synchronous
> +       Replication Support for Logical Decoding</link> for the details.)
>
> So, in v05, I've fixed this to point out the caveats directly.
>
> +       [user] catalog tables exclusively. To avoid this users must refrain from
> +       having locks on catalog tables (e.g. explicit <command>LOCK</command> command)
> +       in such transactions.
> +       (See <xref linkend="logicaldecoding-synchronous-caveats"/> for the details.)
>
> Kindly have a look at the patch-set.
>

Thanks for the updated patch:
Few comments:
1) We have used Reordering and Clustering for the same command, we
could rephrase similarly in both places.
+       <para>
+        Reordering <structname>pg_class</structname> by
<command>CLUSTER</command>
+        command in a transaction.
+       </para>

+       <para>
+        Clustering <structname>pg_trigger</structname> and decoding
<command>PREPARE
+        TRANSACTION</command>, if any published table have a trigger and any
+        operations that will be decoded are conducted.
+       </para>
+      </listitem>

2) Here user_catalog_table should be user catalog table
+       <para>
+        Executing <command>TRUNCATE</command> on user_catalog_table
in a transaction.
+       </para>

Regards,
Vignesh