Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

On Sun, Oct 24, 2021 at 3:15 AM Jeff Davis <pgsql@j-davis.com> wrote:

Add new predefined role pg_maintenance, which can issue VACUUM,
ANALYZE, CHECKPOINT.

Patch attached.

At this point, the idea of having a new role for maintenance work
looks good. With this patch and Mark Dilger's patch introducing a
bunch of new predefined roles, one concern is that we might reach to a
state where we will have patches being proposed for new predefined
roles for every database activity and the superuser eventually will
have nothing to do in the database, it just becomes dummy?

I'm not sure if Mark Dilger's patch on new predefined roles has a
suitable/same role that we can use here.

Are there any other database activities that fall under the
"maintenance" category? How about CLUSTER, REINDEX? I didn't check the
code for their permissions.

Regards,
Bharath Rupireddy.

On Sun, Oct 24, 2021 at 7:49 AM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:

On Sun, Oct 24, 2021 at 3:15 AM Jeff Davis <pgsql@j-davis.com> wrote:

Add new predefined role pg_maintenance, which can issue VACUUM,
ANALYZE, CHECKPOINT.

Are there any other database activities that fall under the
"maintenance" category? How about CLUSTER, REINDEX? I didn't check the
code for their permissions.

I would not lump the I/O intensive cluster and reindexing commands, and
vacuum full, into the same permission bucket as vacuum and analyze.
Checkpoint fits in the middle of that continuum. However, given that both
vacuum and analyze are run to ensure good planner statistics during normal
usage of the database, while the others, including checkpoint, either are
non-normal usage or don't influence the planner, I would shift checkpoint
to the same permission that covers cluster and reindex.

David J.

On Sun, 2021-10-24 at 20:19 +0530, Bharath Rupireddy wrote:

At this point, the idea of having a new role for maintenance work
looks good. With this patch and Mark Dilger's patch introducing a
bunch of new predefined roles, one concern is that we might reach to
a
state where we will have patches being proposed for new predefined
roles for every database activity and the superuser eventually will
have nothing to do in the database, it just becomes dummy?

The idea is that, in different environments, the notion of an
"administrator" should have different capabilities and different risks.
By making the privileges more fine-grained, we enable those different
use cases.

I don't see it as necessarily a problem if superuser doesn't have much
left to do.

I'm not sure if Mark Dilger's patch on new predefined roles has a
suitable/same role that we can use here.

I didn't see one. I think one of the most common reasons to do manual
checkpoints and vacuums is for performance testing, so another
potential name might be "pg_performance". But "pg_maintenance" seemed a
slightly better fit.

Are there any other database activities that fall under the
"maintenance" category? How about CLUSTER, REINDEX? I didn't check
the
code for their permissions.

I looked around and didn't see much else to fit into this category.
CLUSTER and REINDEX are a little too specific for a generic maintenance
operation -- it's unlikely that you'd want to perform those expensive
operations just to tidy up. But if you think something else should fit,
let me know.

Thank you,
Jeff Davis

On 10/24/21, 10:20 AM, "Jeff Davis" <pgsql@j-davis.com> wrote:

On Sun, 2021-10-24 at 20:19 +0530, Bharath Rupireddy wrote:

Are there any other database activities that fall under the
"maintenance" category? How about CLUSTER, REINDEX? I didn't check
the
code for their permissions.

I looked around and didn't see much else to fit into this category.
CLUSTER and REINDEX are a little too specific for a generic maintenance
operation -- it's unlikely that you'd want to perform those expensive
operations just to tidy up. But if you think something else should fit,
let me know.

My initial reaction was that members of pg_maintenance should be able
to do all of these things (VACUUM, ANALYZE, CLUSTER, REINDEX, and
CHECKPOINT). It's true that some of these are more expensive or
disruptive than others, but how else could we divvy it up? Maybe one
option is to have two separate roles, one for commands that require
lower lock levels (i.e., ANALYZE and VACUUM without TRUNCATE and
FULL), and another for all of the maintenance commands.

Nathan

On Sun, 2021-10-24 at 21:32 +0000, Bossart, Nathan wrote:

My initial reaction was that members of pg_maintenance should be able
to do all of these things (VACUUM, ANALYZE, CLUSTER, REINDEX, and
CHECKPOINT).

What about REFRESH MATERIALIZED VIEW? That seems more specific to a
workload, but it's hard to draw a clear line between that and CLUSTER.

Maybe one
option is to have two separate roles, one for commands that require
lower lock levels (i.e., ANALYZE and VACUUM without TRUNCATE and
FULL), and another for all of the maintenance commands.

My main motivation is CHECKPOINT and database-wide VACUUM and ANALYZE.
I'm fine extending it if others think it would be worthwhile, but it
goes beyond my use case.

Regards,
Jeff Davis

On 10/24/21, 11:13 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:

On Sun, 2021-10-24 at 21:32 +0000, Bossart, Nathan wrote:

My initial reaction was that members of pg_maintenance should be able
to do all of these things (VACUUM, ANALYZE, CLUSTER, REINDEX, and
CHECKPOINT).

What about REFRESH MATERIALIZED VIEW? That seems more specific to a
workload, but it's hard to draw a clear line between that and CLUSTER.

Hm. CLUSTER reorders the content of a table but does not change it.
REFRESH MATERIALIZED VIEW, on the other hand, does replace the
content. I think that's the sort of line I'd draw between REFRESH
MATERIALIZED VIEW and the other commands as well, so I'd leave it out
of pg_maintenance.

Nathan

Greetings,

* Bharath Rupireddy (bharath.rupireddyforpostgres@gmail.com) wrote:

On Sun, Oct 24, 2021 at 3:15 AM Jeff Davis <pgsql@j-davis.com> wrote:

Add new predefined role pg_maintenance, which can issue VACUUM,
ANALYZE, CHECKPOINT.

Patch attached.

At this point, the idea of having a new role for maintenance work
looks good. With this patch and Mark Dilger's patch introducing a
bunch of new predefined roles, one concern is that we might reach to a
state where we will have patches being proposed for new predefined
roles for every database activity and the superuser eventually will
have nothing to do in the database, it just becomes dummy?

Independent of other things, getting to the point where everything can
be done in the database without the need for superuser is absolutely a
good goal to be striving for, not something to be avoiding.

I don't think that makes superuser become 'dummy', but perhaps the
only explicit superuser check we end up needing is "superuser is a
member of all roles". That would be a very cool end state.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

Independent of other things, getting to the point where everything can
be done in the database without the need for superuser is absolutely a
good goal to be striving for, not something to be avoiding.
I don't think that makes superuser become 'dummy', but perhaps the
only explicit superuser check we end up needing is "superuser is a
member of all roles". That would be a very cool end state.

I'm not entirely following how that's going to work. It implies that
there is some allegedly-not-superuser role that has the ability to
become superuser -- either within SQL or by breaking out to the OS --
because certainly a superuser can do those things.

I don't think we're serving any good purpose by giving people the
impression that roles with such permissions are somehow not
superuser-equivalent. Certainly, the providers who don't want to
give users superuser are just going to need a longer list of roles
they won't give access to (and they probably won't be pleased about
having to vet every predefined role carefully).

regards, tom lane

Greetings,

* Jeff Davis (pgsql@j-davis.com) wrote:

On Sun, 2021-10-24 at 21:32 +0000, Bossart, Nathan wrote:

My initial reaction was that members of pg_maintenance should be able
to do all of these things (VACUUM, ANALYZE, CLUSTER, REINDEX, and
CHECKPOINT).

What about REFRESH MATERIALIZED VIEW? That seems more specific to a
workload, but it's hard to draw a clear line between that and CLUSTER.

Let's not forget that there are already existing non-superusers who can
run things like REFRESH MATERIALIZED VIEW- the owner.

Maybe one
option is to have two separate roles, one for commands that require
lower lock levels (i.e., ANALYZE and VACUUM without TRUNCATE and
FULL), and another for all of the maintenance commands.

My main motivation is CHECKPOINT and database-wide VACUUM and ANALYZE.
I'm fine extending it if others think it would be worthwhile, but it
goes beyond my use case.

I've been wondering what the actual use-case here is. DB-wide VACUUM
and ANALYZE are already able to be run by the database owner, but
probably more relevant is that DB-wide VACUUMs and ANALYZEs shouldn't
really be necessary given autovacuum, so why are we adding predefined
roles which will encourage users to do that?

I was also contemplating a different angle on this- allowing users to
request autovacuum to run vacuum/analyze on a particular table. This
would have the advantage that you get the vacuum/analyze behavior that
autovacuum has (giving up an attempted truncate lock if another process
wants a lock on the table, going at a slower pace rather than going all
out and sucking up lots of I/O, etc).

I'm not completely against this approach but just would like a bit
better understanding of why it makes sense and what things we'll be able
to say about what this role can/cannot do.

Lastly though- I dislike the name, it seems far too general. I get that
naming things is hard but maybe we could find something better than
'pg_maintenance' for this.

Thanks,

Stephen

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

Independent of other things, getting to the point where everything can
be done in the database without the need for superuser is absolutely a
good goal to be striving for, not something to be avoiding.
I don't think that makes superuser become 'dummy', but perhaps the
only explicit superuser check we end up needing is "superuser is a
member of all roles". That would be a very cool end state.

I'm not entirely following how that's going to work. It implies that
there is some allegedly-not-superuser role that has the ability to
become superuser -- either within SQL or by breaking out to the OS --
because certainly a superuser can do those things.

I don't think it implies that at all. Either that, or I'm not following
what you're saying here. We have predefined roles today which aren't
superusers and yet they're able to break out to the OS. Of course they
can become superusers if they put the effort in. None of that gets away
from the more general idea that we could get to a point where all of a
superuser's capabilities come from roles which the superuser is
automatically a member of such that we need have only one explicit
superuser() check.

I don't think we're serving any good purpose by giving people the
impression that roles with such permissions are somehow not
superuser-equivalent. Certainly, the providers who don't want to
give users superuser are just going to need a longer list of roles
they won't give access to (and they probably won't be pleased about
having to vet every predefined role carefully).

I agree that we need to be clear through the documentation about which
predefined roles are able to "break outside the box" and become
superuser, but that's independent from the question of if we will get to
a point where every capability the superuser has can also be given
through membership in some predefined role or not.

That providers have to figure out what makes sense for them in terms of
what they'll allow their users to do or not do doesn't seem entirely
relevant here- different providers are by definition different and some
might be fine with given out certain rights that others don't want to
give out. That's actually kind of the point of breaking out all of
these capabilities into more fine-grained ways of granting capabilities
out.

We already have roles today which are ones that can break outside the
box and get to the OS and are able to do things that a superuser can do,
or become a superuser themselves, and that's been generally a positive
thing. We also have roles which are able to do things that only
superusers used to be able to do but which are not able to become
superusers themselves and aren't able to break out of the box. I don't
see any reason why we can't continue with this and eventually eliminate
the explicit superuser() checks except from the one where a superuser is
a member of all roles. Sure, some of those roles give capabilities
which can be used to become superuser, while others don't, but I hardly
see that as an argument against the general idea that each is able to be
independently GRANT'd.

If nothing else, if we could eventually get to the point where there's
only one such explicit check then maybe we'd stop creating *new* places
where capabilities are locked behind a superuser check. I did an audit
once upon a time of all superuser checks and rather than that number
going down, as I had hoped at the time, it's been going up instead
across new releases. I view that as unfortunate.

Thanks,

Stephen

On Mon, 2021-10-25 at 13:54 -0400, Stephen Frost wrote:

Let's not forget that there are already existing non-superusers who
can
run things like REFRESH MATERIALIZED VIEW- the owner.

Right, that's one reason why I don't see a particular use case there.

But CHECKPOINT right now has an explicit superuser check, and it would
be nice to be able to avoid that.

It's pretty normal to issue a CHECKPOINT right after a data load and
before running a performance test, right? Shouldn't there be some way
to do that without superuser?

Regards,
Jeff Davis

On 2021-Oct-25, Jeff Davis wrote:

But CHECKPOINT right now has an explicit superuser check, and it would
be nice to be able to avoid that.

It's pretty normal to issue a CHECKPOINT right after a data load and
before running a performance test, right? Shouldn't there be some way
to do that without superuser?

Maybe you just need pg_checkpointer.

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/

On Oct 24, 2021, at 7:49 AM, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:

At this point, the idea of having a new role for maintenance work
looks good. With this patch and Mark Dilger's patch introducing a
bunch of new predefined roles, one concern is that we might reach to a
state where we will have patches being proposed for new predefined
roles for every database activity and the superuser eventually will
have nothing to do in the database, it just becomes dummy?

I'm not sure if Mark Dilger's patch on new predefined roles has a
suitable/same role that we can use here.

If you refer to the ALTER SYSTEM SET patches, which I agree introduce a number of new predefined roles, it may interest you that Andrew has requested that I rework that patch set. In particular, he would like me to implement a new system of grants whereby the authority to ALTER SYSTEM SET can be granted per GUC rather than having predefined roles which hardcoded privileges.

I have not withdrawn the ALTER SYSTEM SET patches yet, as I don't know if Andrew's proposal can be made to work, but I wouldn't recommend tying this pg_maintenance idea to that set.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, 2021-10-25 at 17:55 -0300, Alvaro Herrera wrote:

Maybe you just need pg_checkpointer.

Fair enough. Attached simpler patch that only covers checkpoint, and
calls the role pg_checkpointer.

Regards,
Jeff Davis

On 10/25/21, 4:40 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:

On Mon, 2021-10-25 at 17:55 -0300, Alvaro Herrera wrote:

Maybe you just need pg_checkpointer.

Fair enough. Attached simpler patch that only covers checkpoint, and
calls the role pg_checkpointer.

It feels a bit excessive to introduce a new predefined role just for
this. Perhaps this could be accomplished with a new function that
could be granted.

Nathan

On Tue, 2021-10-26 at 00:07 +0000, Bossart, Nathan wrote:

It feels a bit excessive to introduce a new predefined role just for
this. Perhaps this could be accomplished with a new function that
could be granted.

It would be nice if the syntax could be used, since it's pretty
widespread. I guess it does feel excessive to have its own predefined
role, but at the same time it's hard to group a command like CHECKPOINT
into a category. Maybe if we named it something like pg_performance or
something we could make a larger group?

Regards,
Jeff Davis

On 10/25/21, 6:48 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:

On Tue, 2021-10-26 at 00:07 +0000, Bossart, Nathan wrote:

It feels a bit excessive to introduce a new predefined role just for
this. Perhaps this could be accomplished with a new function that
could be granted.

It would be nice if the syntax could be used, since it's pretty
widespread. I guess it does feel excessive to have its own predefined
role, but at the same time it's hard to group a command like CHECKPOINT
into a category. Maybe if we named it something like pg_performance or
something we could make a larger group?

I do think a larger group is desirable, but as is evidenced by this
thread, it may be some time until we can figure out exactly how that
would look. I feel like there's general support for being able to
allow non-superusers to CHECKPOINT and do other
maintenance/performance tasks, though.

I think my main concern with pg_checkpointer is that it could set a
precedent for new predefined roles, and we'd end up with dozens or
more. But as long as new predefined roles aren't terribly expensive,
maybe that's not all that bad. The advantage of having a
pg_checkpointer role is that it enables users to grant just CHECKPOINT
and nothing else. If we wanted a larger "pg_performance" group in the
future, we could introduce that role and make it a member of
pg_checkpointer and others (similar to how pg_monitor works).

Nathan

Greetings,

* Jeff Davis (pgsql@j-davis.com) wrote:

On Tue, 2021-10-26 at 00:07 +0000, Bossart, Nathan wrote:

It feels a bit excessive to introduce a new predefined role just for
this. Perhaps this could be accomplished with a new function that
could be granted.

It would be nice if the syntax could be used, since it's pretty
widespread. I guess it does feel excessive to have its own predefined
role, but at the same time it's hard to group a command like CHECKPOINT
into a category. Maybe if we named it something like pg_performance or
something we could make a larger group?

For the use-case presented, I don't really buy off on this argument.
We're talking about benchmarking tools, surely they can be and likely
already are updated with some regularity for new major versions of PG.
I wonder also if there aren't other things besides this that would need
to be changed for them to work as a non-superuser anyway too, meaning
this would be just one change among others that they'd need to make.

In this specific case, I'd be more inclined to provide a function rather
than an explicit predefined role for this one thing.

Thanks,

Stephen

On Tue, 2021-10-26 at 16:02 -0400, Stephen Frost wrote:

We're talking about benchmarking tools

What I had in mind was something much less formal, like a self-
contained repro case of a performance problem.

... simple schema
... data load
... maybe build some indexes
... maybe set hints
VACUUM ANALYZE;
CHECKPOINT;

I'm not saying it's a very strong use case, but at least for me, it's
kind of a habit to throw in a CHECKPOINT after a quick data load for a
test, even if it might not matter for whatever I'm testing.

I guess I can change my habit to use a function instead, but then
what's the point of the syntax?

Should we just add a builtin function pg_checkpoint(), and deprecate
the syntax?

Regards,
Jeff Davis