Fix for pg_stat_activity putting client hostaddr into appname field
Edmund Horner <ejrh00@gmail.com>
From: Edmund Horner <ejrh00@gmail.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-03-27T02:47:07Z
Lists: pgsql-hackers
Attachments
- pgstats_memory_sizes_v1.patch (application/octet-stream) patch v1
I noticed when querying pg_stat_activity (in 10.1): $ SELECT pid, application_name, client_hostname, backend_type FROM pg_stat_activity; pid | application_name | client_hostname | backend_type -------+--------------------------+-----------------------+------------------------------ 10207 | | | autovacuum launcher 10209 | | | logical replication launcher 10211 | psql | localhost | client backend 10212 | DBeaver 4.3.4 - Main | ms006593.met.co.nz | client backend 10213 | DBeaver 4.3.4 - Metadata | ms006593.met.co.nz | client backend 10219 | psql | at-ice-db01.met.co.nz | client backend 10205 | ms006593.met.co.nz | | background writer 10204 | ms006593.met.co.nz | | checkpointer 10206 | at-ice-db01.met.co.nz | | walwriter I've tracked this down to bootstrap/pgstat.c. We create shared memory segments BackendAppnameBuffer, BackendClientHostnameBuffer, and BackendActivityBufferSize with enough room for MaxBackends strings each. In each PgBackendStatus item, we point st_appname, st_clienthostname, and st_activity_raw to the appropriate offset within this blocks. But the stats array includes auxiliary processes, which means it has NumBackendStatSlots items. The pointers for the aux process strings are out of range. In the case of my query, the pointers for st_appname in the aux processes happen to point into the BackendClientHostnameBuffer segment. This patch uses NumBackendStatSlots for the sizes of those segments, rather than MaxBackends. I considered whether aux processes really need those strings (especially st_clienthostname), but decided it was more consistent just to assume that they might. (It's an extra 3 kB... if we want to save that we can put a bunch of if statements in pgstat_bestart and other places.) The patch also allocates local memory for st_clienthostname in pgstat_read_current_status. These strings shouldn't change very often, but it seems safer and more consistent to treat them as we treat the other two string fields. Without the string copy, I think a client disconnect and be replaced after the stats have been copied, resulting in the new hostname appearing alongside the copied stats fields from the old connection. Cheers, Edmund
Commits
-
Allocate enough shared string memory for stats of auxiliary processes.
- 93b3d43dc188 10.4 landed
- 811969b218ac 11.0 landed
-
Make local copy of client hostnames in backend status array.
- dfc383cf3975 9.3.23 landed
- 310d1379dd71 9.4.18 landed
- fd2efda5d6f1 9.5.13 landed
- 74dc05e01e5a 9.6.9 landed
- 89c2ab340398 10.4 landed
- a820b4c32946 11.0 landed
-
Show more processes in pg_stat_activity.
- fc70a4b0df38 10.0 cited