On Wed, Sep 23, 2026 at 1:25 PM Sami Imseih <[email protected]> wrote: > > > 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.
Thank you for updating the patch! I've pushed the 0001 patch as it looks good to me. I'll review 0002 patch and send review comments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
