| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
| Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Add backendType to PGPROC, replacing isRegularBackend |
| Date: | 2026-02-04 08:23:16 |
| Message-ID: | 5019DFCE-6E86-4A8D-99F9-E2885E51DD6E@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 4, 2026, at 15:17, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Feb 03, 2026 at 03:04:41PM -0600, Nathan Bossart wrote:
>> On Tue, Feb 03, 2026 at 10:55:20PM +0200, Heikki Linnakangas wrote:
>>> I propose a little refactoring, attached, to replace the "isRegularBackend"
>>> field in PGPROC with full "backendType".
>>>
>>> Andres briefly suggested this a while back [1]:
>>>
>>> On Fri, 22 Nov 2024 at 22:13, Andres Freund <andres(at)anarazel(dot)de>
>>> wrote:
>>>> Or we could have a copy of the backend type in PGPROC.
>>>
>>> but we didn't follow up on that approach. I don't see why, it seems so much
>>> simpler than what we ended up doing. Am I missing something?
>>
>> At a glance, it looks reasonable to me. I don't recall whether I explored
>> this approach, but at the very least I'm unaware of any reason it wouldn't
>> work.
>>
>>> @@ -684,7 +684,7 @@ InitAuxiliaryProcess(void)
>>> MyProc->databaseId = InvalidOid;
>>> MyProc->roleId = InvalidOid;
>>> MyProc->tempNamespaceId = InvalidOid;
>>> - MyProc->isRegularBackend = false;
>>> + MyProc->backendType = B_INVALID;
>>> MyProc->delayChkptFlags = 0;
>>> MyProc->statusFlags = 0;
>>> MyProc->lwWaiting = LW_WS_NOT_WAITING;
>>
>> Hm. So for auxiliary processes, this would always be unset? That appears
>> to be alright for today's use-cases, but it could be a problem down the
>> road.
>
> Yeah, and that would contradict what their associated "Main" functions set.
>
> For example, for the checkpointer we now have:
>
> (gdb) p MyProc->backendType
> $1 = B_INVALID
> (gdb) p MyBackendType
> $2 = B_CHECKPOINTER <---- set by CheckpointerMain()
>
> Also one point to notice:
>
> - bool isRegularBackend; /* true if it's a regular backend. */
> + BackendType backendType; /* what kind of process is this? */
>
>
> we're going from 1 byte to 4 bytes and that does not change the struct size (thanks
> to the padding): still 832 bytes. I guess that's good that it is still a multiple
> of 64.
>
> Regards,
>
> --
> Bertrand Drouvot
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
Yeah, I think we should assign MyBackendType to MyProc->backendType.
InitAuxiliaryProcess() is only called by AuxiliaryProcessMainCommon(), and AuxiliaryProcessMainCommon() is always called after setting MyBackendType, for example:
```
void
BackgroundWriterMain(const void *startup_data, size_t startup_data_len)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext bgwriter_context;
bool prev_hibernate;
WritebackContext wb_context;
Assert(startup_data_len == 0);
MyBackendType = B_BG_WRITER;
AuxiliaryProcessMainCommon();
```
Otherwise, the patch looks good to me. I applied the patch locally, both build and “make check” passed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Henson Choi | 2026-02-04 08:54:26 | Re: Row pattern recognition |
| Previous Message | Shlok Kyal | 2026-02-04 08:16:06 | Re: Skipping schema changes in publication |