public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag
Date: Wed, 4 Oct 2023 08:17:41 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACUAR0i1=YG0pXBb3u-6299ny3OBATEe_9Xf8XE0wiCbkA@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CALj2ACWZ1a69K9GMsUpCY+kyMqqeHMWVu+UUAe7=gBCkk1Xp8Q@mail.gmail.com>
	<[email protected]>
	<CALj2ACUAR0i1=YG0pXBb3u-6299ny3OBATEe_9Xf8XE0wiCbkA@mail.gmail.com>

On Tue, Oct 03, 2023 at 07:02:11PM +0530, Bharath Rupireddy wrote:
> On Tue, Oct 3, 2023 at 5:45 PM Drouvot, Bertrand
> <[email protected]> wrote:
>> I did some research and BGWORKER_BYPASS_ALLOWCONN has been added in eed1ce72e1 and
>> at that time the bgw_flags did already exist.
>>
>> In this the related thread [1], Tom mentioned:
>>
>> "
>> We change exported APIs in new major versions all the time.  As
>> long as it's just a question of an added parameter, people can deal
>> with it.
>> "
> 
> It doesn't have to be always/all the time. If the case here is okay to
> change the bgw and other core functions API, I honestly feel that we
> must move BGWORKER_BYPASS_ALLOWCONN to bgw_flags.

I don't agree with this point.  BackgroundWorkerInitializeConnection()
and its other friend are designed to be called at the beginning of the
main routine of a background worker, where bgw_flags is not accessible.
There is much more happening before a bgworker initializes its
connection, like signal handling and definitions of other states that
depend on the GUCs loaded for the bgworker.

>> Now, I understand your point but it looks to me that bgw_flags is more
>> about the capabilities (Access to shared memory with BGWORKER_SHMEM_ACCESS
>> or ability to establish database connection with BGWORKER_BACKEND_DATABASE_CONNECTION),
>>
>> While with BGWORKER_BYPASS_ROLELOGINCHECK (and BGWORKER_BYPASS_ALLOWCONN) it's more related to
>> the BGW behavior once the capability is in place.
> 
> I look at the new flag as a capability of the bgw to connect with a
> role without login access. IMV, all are the same.

Bertrand is arguing that the current code with its current split is
OK, because both are different concepts:
- bgw_flags is used by the postmaster to control how to launch the
bgworkers.
- The BGWORKER_* flags are used by the bgworkers themselves, once
things are set up by the postmaster based on bgw_flags.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (8+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox