don't allow walsender to consume superuser_reserved_connection slots, or during shutdown

Started by Robert Haasalmost 16 years ago5 messageshackers
Jump to latest
#1Robert Haas
robertmhaas@gmail.com

On Wed, Apr 21, 2010 at 1:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

...shouldn't we move the "tests", plural, rather than just the one?
It seems right to reject new SR connections during shutdown.

Yeah; you'd also need to adjust both of them to consider am_walsender.
(IOW, we want to treat SR connections as non-superuser for both tests.)

[ subject changed, recipient list trimmed ]

Here's the fine patch. The actual code changes are simple and seem to
work as expected, but I struggled a bit with the phrasing of the
messages. Feel free to suggest improvements. Also, I wasn't sure if
there was somewhere in the documentation where we discussed the
restriction that only superusers can connect during shutdown. If
there is such a place, we should update that, too.

...Robert

Attachments:

superuser_is_not_enuf.patchapplication/octet-stream; name=superuser_is_not_enuf.patchDownload+33-21
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#1)
Re: don't allow walsender to consume superuser_reserved_connection slots, or during shutdown

Robert Haas <robertmhaas@gmail.com> writes:

Here's the fine patch. The actual code changes are simple and seem to
work as expected, but I struggled a bit with the phrasing of the
messages. Feel free to suggest improvements.

Stick with the original wording? I don't really see a need to change it.

regards, tom lane

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Tom Lane (#2)
Re: don't allow walsender to consume superuser_reserved_connection slots, or during shutdown

On Thu, Apr 22, 2010 at 11:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Here's the fine patch.  The actual code changes are simple and seem to
work as expected, but I struggled a bit with the phrasing of the
messages.  Feel free to suggest improvements.

Stick with the original wording?  I don't really see a need to change it.

How about?:

if ((!am_superuser || am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))
{
if (am_walsender)
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for
non-replication superuser connections")));
else
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("connection limit exceeded for non-superusers")));
}

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#4Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#2)
Re: don't allow walsender to consume superuser_reserved_connection slots, or during shutdown

On Wed, Apr 21, 2010 at 10:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Here's the fine patch.  The actual code changes are simple and seem to
work as expected, but I struggled a bit with the phrasing of the
messages.  Feel free to suggest improvements.

Stick with the original wording?  I don't really see a need to change it.

I don't think that's a good idea. If we just say that the remaining
connection slots are for superusers, someone will inevitably ask us
why their superuser replication can't connect. I think it's important
to phrase things as accurately as possible.

...Robert

#5Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#1)
Re: don't allow walsender to consume superuser_reserved_connection slots, or during shutdown

On Wed, Apr 21, 2010 at 9:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Apr 21, 2010 at 1:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

...shouldn't we move the "tests", plural, rather than just the one?
It seems right to reject new SR connections during shutdown.

Yeah; you'd also need to adjust both of them to consider am_walsender.
(IOW, we want to treat SR connections as non-superuser for both tests.)

[ subject changed, recipient list trimmed ]

Here's the fine patch.  The actual code changes are simple and seem to
work as expected, but I struggled a bit with the phrasing of the
messages.  Feel free to suggest improvements.  Also, I wasn't sure if
there was somewhere in the documentation where we discussed the
restriction that only superusers can connect during shutdown.  If
there is such a place, we should update that, too.

I have committed this as-is. We can further change the error messages
if we like, but there didn't seem to be a clear consensus on any
particular change from what I have here.

...Robert