On 7/24/21, 9:16 AM, "Bharath Rupireddy" <bharath.rupireddyforpostg...@gmail.com> wrote: > On Fri, Jul 23, 2021 at 4:40 AM Bossart, Nathan <bossa...@amazon.com> wrote: >> I would suggest changing "attach from a debugger" to "attaching with a >> debugger." > > Thanks. IMO, the following looks better: > <entry>Waiting on connection startup before authentication to allow > attaching a debugger to the process.</entry> > <entry>Waiting on connection startup after authentication to allow > attaching a debugger to the process.</entry>
Your phrasing looks good to me. >> IIUC you want to use the same set of flags as PostAuthDelay for >> PreAuthDelay, but the stated reason in this comment for leaving out >> WL_LATCH_SET suggests otherwise. It's not clear to me why the latch >> possibly pointing to a shared latch in the future is an issue. Should >> this instead say that we leave out WL_LATCH_SET for consistency with >> PostAuthDelay? > > If WL_LATCH_SET is used for PostAuthDelay, the waiting doesn't happen > because the MyLatch which is a shared latch would be set by > SwitchToSharedLatch. More details at [1]. > If WL_LATCH_SET is used for PreAuthDelay, actually there's no problem > because MyLatch is still not initialized properly in BackendInitialize > when waiting for PreAuthDelay, it still points to local latch, but > later gets pointed to shared latch and gets set SwitchToSharedLatch. > But relying on MyLatch there seems to me somewhat relying on an > uninitialized variable. More details at [1]. > > For PreAuthDelay, with the comment I wanted to say that the MyLatch is > not the correct one we would want to wait for. Since there is no > problem in using it there, I changed the comment to following: > /* > * Let's not use WL_LATCH_SET for PreAuthDelay to be consistent with > * PostAuthDelay. > */ How about we elaborate a bit? WL_LATCH_SET is not used for consistency with PostAuthDelay. MyLatch isn't fully initialized for the backend at this point, anyway. + /* + * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This + * is because the latch could have been set initially. + */ I would suggest the following: If WL_LATCH_SET is used, PostAuthDelay may not be applied, since the latch might already be set. Otherwise, this patch looks good and could probably be marked ready- for-committer. Nathan