Date: Tue, 6 Dec 2022 11:25:51 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Melih Mutlu <m.melihmutlu@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>,
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: wake up logical workers after ALTER SUBSCRIPTION
Message-ID: <20221206192551.GA3078082@nathanxps13>
References: <20221123205027.GA368966@nathanxps13>
 <TYCPR01MB58703BC1BD6B1415CA4BAB7FF50F9@TYCPR01MB5870.jpnprd01.prod.outlook.com>
 <20221127234528.GB878043@nathanxps13>
 <20221130041028.GA1604177@nathanxps13>
 <CA+hUKGJTQqnAAYAmdM9xkcs1e+5jOt34nvV622cY8cTmQcgVpA@mail.gmail.com>
 <CA+hUKGJXO6LUn3wTf3s0Z8DvgbQROxPTnJDHCsQrhbr3pF3g8Q@mail.gmail.com>
 <20221130050441.GA1677223@nathanxps13>
 <20221202002130.GA2124877@nathanxps13>
 <20221202192101.GB2277157@nathanxps13>
 <CAGPVpCTUuKyxB5f3exgDMBRTivbN5uHVkD=YfXmgznrdnwVOQw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="LZvS9be/3tNcYl/X"
Content-Disposition: inline
In-Reply-To: 
 <CAGPVpCTUuKyxB5f3exgDMBRTivbN5uHVkD=YfXmgznrdnwVOQw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20221206192551.GA3078082%40nathanxps13>
Precedence: bulk


--LZvS9be/3tNcYl/X
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Thanks for reviewing!

On Tue, Dec 06, 2022 at 07:44:46PM +0300, Melih Mutlu wrote:
> Is it really necessary to wake logical workers up when renaming other than
> subscription or publication? address.objectId will be a valid subid only
> when renaming a subscription.

Oops, that is a mistake.  I only meant to wake up the workers for ALTER
SUBSCRIPTION RENAME.  I think I've fixed this in v6.

> - When the state is SYNCDONE and the apply worker has to wake up to change
> the state to READY.
> 
> I think we already call logicalrep_worker_wakeup_ptr wherever it's needed
> for the above cases? What am I missing here?

IIUC we must restart all the apply workers for a subscription to enable
two_phase mode.  It looks like finish_sync_worker() only wakes up its own
apply worker.  I moved this logic to where the sync worker marks the state
as SYNCDONE and added a check that two_phase mode is pending.  Even so,
there can still be unnecessary wakeups, but this adjustment should limit
them.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

--LZvS9be/3tNcYl/X
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v6-0001-wake-up-logical-workers-as-needed-instead-of-rely.patch"