Re: Allowing REINDEX to have an optional name

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Allowing REINDEX to have an optional name
Date: 2022-05-11 04:24:17
Message-ID: CAGEoWWTg+BcoHU-mB1izV6c6CrNVk43zt2aaFaH5C=oGVR5bVg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 10, 2022 at 7:30 PM Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>
wrote:

> On Tue, 10 May 2022 at 14:47, Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >
> > On Tue, May 10, 2022 at 2:43 PM Simon Riggs
> > <simon(dot)riggs(at)enterprisedb(dot)com> wrote:
> > >
> > > A minor issue, and patch.
> > >
> > > REINDEX DATABASE currently requires you to write REINDEX DATABASE
> > > dbname, which makes this a little less usable than we might like.
> > >
> > > REINDEX on the catalog can cause deadlocks, which also makes REINDEX
> > > DATABASE not much use in practice, and is the reason there is no test
> > > for REINDEX DATABASE. Another reason why it is a little less usable
> > > than we might like.
> > >
> > > Seems we should do something about these historic issues in the name
> > > of product usability.
> > >
> > > Attached patch allows new syntax for REINDEX DATABASE, without needing
> > > to specify dbname. That version of the command skips catalog tables,
> > > as a way of avoiding the known deadlocks. Patch also adds a test.
> > >
> >
> > From the patch it looks like with the patch applied running REINDEX
> > DATABASE is equivalent to running REINDEX DATABASE <current database>
> > except reindexing the shared catalogs. Is that correct?
>
> Yes
>
> > Though the patch adds following change
> > + Indexes on shared system catalogs are also processed, unless the
> > + database name is omitted, in which case system catalog indexes
> > are skipped.
> >
> > the syntax looks unintuitive.
> >
> > I think REINDEX DATABASE reindexing the current database is a good
> > usability improvement in itself. But skipping the shared catalogs
> > needs an explicity syntax. Not sure how feasible it is but something
> > like REINDEX DATABASE skip SHARED/SYSTEM.
>
> There are two commands:
>
> REINDEX DATABASE does every except system catalogs
> REINDEX SYSTEM does system catalogs only
>

IIUC

REINDEX DATABASE <database name> does system catalogs as well
REINDEX DATABASE does everything except system catalogs

That's confusing and unintuitive.

Not providing the database name leads to ignoring system catalogs. I won't
expect that from this syntax.

> So taken together, the two commands seem intuitive to me.
>
> It is designed like this because it is dangerous to REINDEX the system
> catalogs because of potential deadlocks, so we want a way to avoid
> that problem.
>

It's more clear if we add SKIP SYSTEM CATALOGS or some such explicit syntax.

--
Best Wishes,
Ashutosh

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2022-05-11 04:45:24 Re: Support logical replication of DDLs
Previous Message Nathan Bossart 2022-05-11 04:12:11 Re: make MaxBackends available in _PG_init