Re: almost-super-user problems that we haven't fixed yet

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-01-18T16:28:57Z
Lists: pgsql-hackers
On Tue, Jan 17, 2023 at 7:15 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> Great.  Here is a first attempt at the patch.

In general, looks good. I think this will often call HaveNFreeProcs
twice, though, and that would be better to avoid, e.g.

if (!am_superuser && !am_walsender && (SuperuserReservedBackends +
ReservedBackends) > 0)
    && !HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends))
{
    if (!HaveNFreeProcs(SuperuserReservedBackends))
        remaining connection slots are reserved for non-replication
superuser connections;
    if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS))
       remaining connection slots are reserved for roles with
privileges of pg_use_reserved_backends;
}

In the common case where we hit neither limit, this only counts free
connection slots once. We could do even better by making
HaveNFreeProcs have an out parameter for the number of free procs
actually found when it returns false, but that's probably not
important.

I don't think that we should default both the existing GUC and the new
one to 3, because that raises the default limit in the case where the
new feature is not used from 3 to 6. I think we should default one of
them to 0 and the other one to 3. Not sure which one should get which
value.

> > I think the documentation will need some careful wordsmithing,
> > including adjustments to superuser_reserved_connections. We want to
> > recast superuser_reserved_connections as a final reserve to be touched
> > after even reserved_connections has been exhausted.
>
> I tried to do this, but there is probably still room for improvement,
> especially for the parts that discuss the relationship between
> max_connections, superuser_reserved_connections, and reserved_connections.

I think it's pretty good the way you have it. I agree that there might
be a way to make it even better, but I don't think I know what it is.

-- 
Robert Haas
EDB: http://www.enterprisedb.com



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Improve several permission-related error messages.

  2. Integrate superuser check into has_rolreplication()

  3. Small code simplification

  4. Adjust interaction of CREATEROLE with role properties.

  5. Add new GUC reserved_connections.

  6. Rename ReservedBackends variable to SuperuserReservedConnections.

  7. Update docs and error message for superuser_reserved_connections.

  8. Add new GUC createrole_self_grant.

  9. Restrict the privileges of CREATEROLE users.

  10. Add a SET option to the GRANT command.