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 →
-
Improve several permission-related error messages.
- de4d456b406b 16.0 landed
-
Integrate superuser check into has_rolreplication()
- 442f8700656b 16.0 landed
-
Small code simplification
- 3b7cd8c690f2 16.0 landed
-
Adjust interaction of CREATEROLE with role properties.
- f1358ca52dd7 16.0 landed
-
Add new GUC reserved_connections.
- 6e2775e4d4e4 16.0 landed
-
Rename ReservedBackends variable to SuperuserReservedConnections.
- fe00fec1f5d7 16.0 landed
-
Update docs and error message for superuser_reserved_connections.
- 6c1d5ba48678 16.0 landed
-
Add new GUC createrole_self_grant.
- e5b8a4c098ad 16.0 cited
-
Restrict the privileges of CREATEROLE users.
- cf5eb37c5ee0 16.0 cited
-
Add a SET option to the GRANT command.
- 3d14e171e9e2 16.0 cited