From: | "Bossart, Nathan" <bossartn(at)amazon(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Use WaitLatch for {pre, post}_auth_delay instead of pg_usleep |
Date: | 2021-07-26 17:33:03 |
Message-ID: | 9987F198-E3DA-4683-8D45-3606E4F61649@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 7/24/21, 9:16 AM, "Bharath Rupireddy" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> On Fri, Jul 23, 2021 at 4:40 AM Bossart, Nathan <bossartn(at)amazon(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-07-26 17:33:29 | Re: Skip temporary table schema name from explain-verbose output. |
Previous Message | Stephen Frost | 2021-07-26 17:32:31 | Re: needless complexity in StartupXLOG |