RE: wake up logical workers after ALTER SUBSCRIPTION

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Nathan Bossart <nathandbossart@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Melih Mutlu <m.melihmutlu@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-01-24T02:55:07Z
Lists: pgsql-hackers

Attachments

On Monday, January 23, 2023 3:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hi,

> 
> Nathan Bossart <nathandbossart@gmail.com> writes:
> > On Tue, Jan 10, 2023 at 10:59:14AM +0530, Amit Kapila wrote:
> >> I haven't looked in detail but isn't it better to explain somewhere
> >> in the comments that it achieves to rate limit the restart of workers
> >> in case of error and allows them to restart immediately in case of
> >> subscription parameter change?
> 
> > I expanded one of the existing comments to make this clear.
> 
> I pushed v17 with some mostly-cosmetic changes, including more comments.

I noticed one minor thing in this commit. 

-
LogicalRepCtx->last_start_dsh = DSM_HANDLE_INVALID;
-

The code takes the last_start_dsh as dsm_handle, but it seems it is a dsa_pointer.
" typedef dsa_pointer dshash_table_handle;" This won’t cause any problem, but I feel
It would be easier to understand if we take it as dsa_pointer and use InvalidDsaPointer here,
like what he attached patch does. What do you think ?

Best regards,
Hou zj



Commits

  1. Avoid type cheats for invalid dsa_handles and dshash_table_handles.

  2. Track logrep apply workers' last start times to avoid useless waits.

  3. Check for two_phase change at end of process_syncing_tables_for_apply.

  4. Wake up a subscription's replication worker processes after DDL.