Обсуждение: Add backendType to PGPROC, replacing isRegularBackend
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? [1] https://www.postgresql.org/message-id/ujenaa2uabzfkwxwmfifawzdozh3ljr7geozlhftsuosgm7n7q%40g3utqqyyosb6 - Heikki
Вложения
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. -- nathan
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
> On Feb 4, 2026, at 15:17, Bertrand Drouvot <bertranddrouvot.pg@gmail.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 calledafter 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/
On 03/02/2026 23:04, 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. Ok. >> @@ -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. Right, that was just a silly mistake. Fixed that and pushed, thanks! - Heikki