> I've reviewed the 0001 patch first as it's an open item. There are
> some minor comments:

Thanks!

> -       expected to be very brief.
> +       expected to be very brief, except for
> +       <command>REPACK (CONCURRENTLY)</command>, where it also covers 
> enabling
> +       logical decoding and then initializing it.  Enabling has nothing to do
> +       if <xref linkend="guc-effective-wal-level"/> is already
> +       <literal>logical</literal>; otherwise it waits for all backends to
> +       acknowledge that they started writing the additional WAL information
> +       that logical decoding requires.  Initializing then waits for the
>
> The logical decoding activation waits for all processes to acknowledge
> it, not only backends.

Fair. I updated it.

> +       transactions that had already been assigned a transaction ID to end, 
> so
> +       this phase can last as long as the longest of those transactions.
>
> During the logical decoding initialization, the snapshot builder also
> has to wait for transactions that started after the initialization
> began (i.e., those that started while it was in the BUILDING_SNAPSHOT
> state). So this phase can last longer than the longest of the
> transactions that
> were running when it started. How about rephrasing it as follows?
>
>         expected to be very brief, except for
>         <command>REPACK (CONCURRENTLY)</command>, where it also covers 
> enabling
>         logical decoding and then initializing it.  Enabling has nothing to do
>         if <xref linkend="guc-effective-wal-level"/> is already
>         <literal>logical</literal>; otherwise it waits for all processes to
>         acknowledge that they started writing the additional WAL information
>         that logical decoding requires.  Initializing then waits for the
>         transactions that have been assigned a transaction ID to end,
>         including ones that started while this wait was in progress, so this
>         phase can last longer than the longest transaction running when the
>         command started.

Good point. I used what you provided and I realized that in my v1 I was also
specifically calling out that "waits for the transactions that have
been assigned a transaction ID to end"
also includes other REPACK CONCURRENTLY operations. This may not be obvious,
so it's worth a callout.

--
Sami Imseih
Amazon Web Services (AWS)

Attachment: v4-0002-Add-REPACK-progress-phases-for-logical-decoding-s.patch
Description: Binary data

Attachment: v4-0001-doc-Improve-REPACK-phase-documentation-for-long-o.patch
Description: Binary data

Reply via email to