Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX - Mailing list pgsql-hackers

On Mon, Apr 28, 2025 at 7:23 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
> On Thu, Apr 24, 2025 at 12:45 AM jian he <jian.universality@gmail.com> wrote:
>>
> Thank you for the feedback. I have also updated the feedback from [1] as well. Few extra notes:
>
> - Attached v18
> - Rebased against master
> - Updated the commit message
> - Updated the target remote version to now be fout->remoteVersion >= 190000
> - Using a UNION ALL query to show all indexes from part_tbl partitioned tables in the specs as noted in [1]. The
querysuggested in [1] wasn't encompassing all the indexes, hence the UNION ALL for WHERE i.indrelid =
'part_tbl'::regclass::oid.
>

Having looked at this patch, I'm a bit surprised that it would be
considered for commit; not that the work hasn't been done with rigor,
but the implementation seems extremely obtuse for the common use cases
that have been envisioned.

As a primary example, imagine you have 2 indexes and you want to test
if one index can handle the load via skip scans. With this feature, in
order to do that SAFELY, you would need to first figure out how to
ensure that the `force_invisible_index` GUC has been set to true
across all possible backends, even though there seems like a general
agreement that there isn't an easy way to do this (see comments around
cached plans), and to make it more complicated, this needs to occur
across all downstream replicas. Only then would it be safe to run the
alter index to set your index invisible, at which point you could then
test at the query/session level to determine which queries will be
supportable without the risk of having your server(s) tank due to
overload when you start getting hundreds of queries who plan has gone
sideways. Ideally you would be able to do this in the opposite
fashion; start with a session level guc that allows you to test in a
controlled manner, and then if that works you start to roll that out
across multiple sessions, and then to multiple servers, before
eventually dropping the index.

But that isn't the only gap; imagine if you want to test across 3 or
more indexes; with this implementation, the "use invisible" flag is
all or nothing, which again makes it difficult to work with;
especially if you have multiple cases within the system that might
make use of this feature (and people will surely run invisible indexes
for weeks in production to ensure some random monthly report doesn't
come along and cause trouble). I'm also skeptical of the idea that
users need a way to add invisible indexes they can then test to see if
they are useful because 1) this is basically how indexes already work,
meaning if you add an index and it isn't useful, it doesn't get used,
and 2) we have an extension (hypopg) which arguably provides this
functionality without causing a bunch of i/o, and there isn't nearly
the clamor to add this functionality in to core as there is for having
a way to "soft drop" indexes. TBH, with this implementation, I can see
people running with all indexes set invisible and
force_invisible_index set to true, just to enable simple granular
control when they need it.

I know this thread is rather old and there doesn't seem to be full
agreement on the ALTER vs GUC implementation idea, and even though I
agree with the sentiment that the GUC system is little more than the
"half-baked take on planner hints", the upside of GUC first
implementations is that they tend to provide better usability than
most grammer related implementations. Consider that any implementation
which requires the use of ALTER statements (which this one does)
undercuts its own usefulness because it adds significant operational
risk in any attempt to use it just by the nature of ALTER leading to
system-wide (including multi-server) changes, and while it feels like
we often dismiss operational risk, those are exactly the folks who
need this feature the most.

P.S. I really do want to thank Shayon for sticking with this; I
thought about saying that up front but it felt cliche, but I do think
it is important to say it.

Robert Treat
https://xzilla.net