On 29/03/18 10:46, Michael Paquier wrote:
On Tue, Mar 27, 2018 at 03:47:07PM +1300, Edmund Horner wrote:
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.)
BackendStatusShmemSize has been originally changed to use
NumBackendStatSlots, so the intention is visibly to be consistent in the
number of backends used, even if this means consuming a bit more memory,
so the idea was to focus on consistency and simplicity.
Yep, agreed.
I've committed this, and backpatched to v10.
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.
Agreed on that as well.
Yeah, that's also a bug. I pushed your fix for that as a separate
commit, and backpatched to all supported versions.
Thanks for the debugging and the patch, Edmund!
- Heikki
