Re: Make default subscription streaming option as Parallel

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-10-23T00:27:33Z
Lists: pgsql-hackers

Attachments

Hi Vignesh, here are some review comments for the patch v5-0001 (docs only).

======
Commit message

1.
The commit message only refers to this as the "streaming option", but
an option of what? Somewhere we should mention this is an option of
CREATE SUBSCRIPTION.

~

2.
Previously the default value of this option was 'off'. The parallel option
indicates that the changes in large transactions (greater than
logical_decoding_work_mem) are to be applied directly via one of the
parallel apply workers, if available.

~

typo - /parallel option/'parallel' mode/
typo - /parallel mode/'parallel' mode/
typo - /we refrain from enabling it/we refrained from enabling it/

======
doc/src/sgml/ref/create_subscription.sgml

3.
          <para>
           Specifies whether to enable streaming of in-progress transactions
-          for this subscription.  The default value is <literal>off</literal>,
-          meaning all transactions are fully decoded on the publisher and only
-          then sent to the subscriber as a whole.
+          for this subscription.  The default value is
<literal>parallel</literal>,
+          meaning incoming changes are directly applied via one of the parallel
+          apply workers, if available. If no parallel apply worker is free to
+          handle streaming transactions then the changes are written to
+          temporary files and applied after the transaction is committed. Note
+          that if an error happens in a parallel apply worker, the finish LSN
+          of the remote transaction might not be reported in the server log.
          </para>

The other enum values have separate paragraphs:
- "If set to 'on'" and
- "If set to 'off'"

I felt the 'parallel' value description should have this same style --
e.g. a separate paragraph saying:
- "If set to 'parallel' (the default value)...".

IMO, doing this makes the 3 available enum values much clearer.

Please take a look at the attachment where I've made this suggested change.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Commits

  1. Change the default value of the streaming option to 'parallel'.