> On 5 Sep 2023, at 12:21, Jelte Fennema <m...@jeltef.nl> wrote: > > On Tue, 5 Sept 2023 at 11:39, Daniel Gustafsson <dan...@yesql.se> wrote: >> >>> On 5 Sep 2023, at 09:09, Nishant Sharma <nishant.sha...@enterprisedb.com> >>> wrote: >> >>> With this, I was thinking, isn't this a problem of pgbouncer filling >>> be_pid with random bits? Maybe it should have filled the be_pid >>> with a random positive integer instead of any integer as it >>> represents a pid? >> >> I'm inclined to agree that anyone sending a value which is supposed to >> represent a PID should be expected to send a value which corresponds to the >> format of a PID. > > When there is a pooler in the middle it already isn't a PID anyway. I > took a look at a few other connection poolers and all the ones I > looked at (Odyssey and pgcat) do the same: They put random bytes in > the be_pid field (and thus can result in negative values). This normally > does not cause any problems, because the be_pid value is simply sent > back verbatim to the server when canceling a query, which is it's main > purpose according to the docs: > >> This message provides secret-key data that the frontend must save if it >> wants to be able to issue cancel requests later. > > Source: > https://www.postgresql.org/docs/current/protocol-flow.html#id-1.10.6.7.3 > > For that purpose it's actually more secure to use all bits for random > data, instead of keeping one bit always 0.
If it's common practice to submit a pid which isn't a pid, I wonder if longer term it's worth inventing a value for be_pid which means "unknown pid" such that consumers can make informed calls when reading it? Not the job of this patch to do so, but maybe food for thought. > So, while I agree that putting a negative value in the process ID field of > BackendData, is arguably incorrect. Given the simplicity of the fix on > the pg_basebackup side, I think addressing it in pg_basebackup is > easier than fixing this in all connection poolers. Since the value in the temporary slotname isn't used to convey meaning, but merely to ensure uniqueness, I don't think it's unreasonable to guard aginst malformed input (ie negative integer). -- Daniel Gustafsson
