On 7/24/21, 9:16 AM, "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com> wrote:
> On Fri, Jul 23, 2021 at 4:40 AM Bossart, Nathan <bossartn@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
