Date: Tue, 24 Jan 2023 09:13:29 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, 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>
Subject: Re: wake up logical workers after ALTER SUBSCRIPTION
Message-ID: <20230124171329.GA677220@nathanxps13>
References: 
 <CAA4eK1JFgvhxV448uNTkKpQpGe+X8WRWtyf45h4vASEpNz3Gkw@mail.gmail.com>
 <20230105171933.GB299579@nathanxps13>
 <CAA4eK1L-N9aUpoCXhoBCiEJAGw3cmKxaxn0Fo43ja+qzAGeyhQ@mail.gmail.com>
 <20230106054017.GA786807@nathanxps13>
 <3355444.1673044286@sss.pgh.pa.us>
 <20230107004525.GB789289@nathanxps13>
 <CAA4eK1+2VbK8BCrG7xFKJQKhYhYwWwkddAN2gL2coUQnTKLJ3g@mail.gmail.com>
 <20230110174345.GA1292607@nathanxps13>
 <1353772.1674414774@sss.pgh.pa.us>
 <OS0PR01MB5716860B1454C34E5B179B6694C99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <OS0PR01MB5716860B1454C34E5B179B6694C99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20230124171329.GA677220%40nathanxps13>
Precedence: bulk

On Tue, Jan 24, 2023 at 02:55:07AM +0000, houzj.fnst@fujitsu.com wrote:
> 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 ?

IMO ideally there should be a DSA_HANDLE_INVALID and DSHASH_HANDLE_INVALID
for use with dsa_handle and dshash_table_handle, respectively.  But your
patch does seem like an improvement.

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