Hi Jelte,
Please find my reviews below:- *1)* With what I have understood from above, the pgbouncer fills up be_pid (int, 32 bits) with random bits as it does not have an associated server connection yet. 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? -- If this makes sense here, then maybe the fix should be in pgbouncer instead of how the be_pid is processed in pg_basebackup? *2)* Rest, the patch looks straightforward, with these two changes : "%d" --> "%u" and "(int)" --> "(unsigned)". Regards, Nishant. On Thu, Aug 31, 2023 at 2:43 PM Jelte Fennema <m...@jeltef.nl> wrote: > With PgBouncer in the middle PQbackendPID can return negative values > due to it filling all 32 bits of the be_pid with random bits. > > When this happens it results in pg_basebackup generating an invalid slot > name (when no specific slot name is passed in) and thus throwing an > error like this: > > pg_basebackup: error: could not send replication command > "CREATE_REPLICATION_SLOT "pg_basebackup_-1201966863" TEMPORARY > PHYSICAL ( RESERVE_WAL)": ERROR: replication slot name > "pg_basebackup_-1201966863" contains invalid character > HINT: Replication slot names may only contain lower case letters, > numbers, and the underscore character. > > This patch fixes that problem by formatting the result from PQbackendPID > as an unsigned integer when creating the temporary replication slot > name. > > I think it would be good to backport this fix too. > > Replication connection support for PgBouncer is not merged yet, but > it's pretty much ready: > https://github.com/pgbouncer/pgbouncer/pull/876 > > The reason PgBouncer does not pass on the actual Postgres backend PID > is that it doesn't have an associated server connection yet when it > needs to send the startup message to the client. It also cannot use > it's own PID, because that would be the same for all clients, since > pgbouncer is a single process. >