>> Updated patch attached. > >A few more comments: > >- why did you add the client address to PgStat_MsgHdr, rather than >PgStat_MsgBestart? It would be nice to avoid sending the >client address >for each and every stats message, as it shouldn't change over the life >of the session.
I guess that's from not reading things carefully enough. I looked for where "backend pid" was transmitted, because I thought that would be a good place to put it. Which might bring up the question, why are things like the backend pid sent in msghdr and not in bestart? (seems bestart is nothing more than the header, which is why I missed it completely) >- I think the backend_client_addr and backend_client_port view columns >should be named "client_addr" and "client_port", respectively. Seems reasonable. >- Is there a reason you need to expose and then call the private >network_in() function, rather than using inet_in(), which is >already public? Not really, that part of the code was a copy from client_addr(). >- Is the backend startup time sensitive information? Considering this >information is available via ps(1), perhaps it would be better >to allow >any user to see any backend's startup time, rather than providing a >false sense of security. Well, ps(1) will show you the client address as well, so the argument can be taken there too. For ps(1) access, you need shell access to the machine. Something at least I would *never* give to people who just "do stuff in the database". That said, I'm not sure that the startup time specifically is sensitive, no. No real rason to hide that, but I'm not sure I buy the argument "considering since it's available via ps(1)" ;-) >- We return (NULL, -1) for the client address/port if connected via a >unix domain socket, and (NULL, NULL) for the address/port if the >selecting user isn't a superuser / the owner of that client >connection. >It seems a little ugly to return NULL for the address in both cases, >since the same value is used for two completely different >meanings. Not >sure of a better convention, though -- just idly complaining :) I first did it with NULL,NULL, before I realised I could not make just that distinction. I guess in theory we could return 0.0.0.0 or 255.255.255.255 (similar for IPv6 of course, don't know offhand what that would be), btu that seems at least as ugly. Another way would be to have yet another column that would be "client address type", but given the options I think the current way is probably the least ugly. >You needn't respin the patch, as I can make the necessary >changes myself >-- I'm just looking for comments on the above before applying. Ok. //Magnus ---------------------------(end of broadcast)--------------------------- TIP 7: don't forget to increase your free space map settings
