Re: Alter index rename concurrently to

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andrey Klychkov <aaklychkov(at)mail(dot)ru>, Victor Yegorov <vyegorov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Alter index rename concurrently to
Date: 2018-07-31 21:10:28
Message-ID: c3d9a44c-2ca1-6726-ad03-ed9c37cc7c29@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 27/07/2018 16:16, Robert Haas wrote:
> With respect to this particular patch, I don't know whether there are
> any hazards of the second type. What I'd check, if it were me, is
> what structures in the index's relcache entry are going to get rebuilt
> when the index name changes. If any of those look like things that
> something that somebody could hold a pointer to during the course of
> query execution, or more generally be relying on not to change during
> the course of query execution, then you've got a problem.

I have investigated this, and I think it's safe. Relcache reloads for
open indexes are already handled specially in RelationReloadIndexInfo().
The only pointers that get replaced there are rd_amcache and
rd_options. I have checked where those are used, and it looks like they
are not used across possible relcache reloads. The code structure in
those two cases make this pretty unlikely even in the future. Also,
violations would probably be caught by CLOBBER_CACHE_ALWAYS.

> As to the
> first category, I suspect it's possible to construct cases where the
> fact that the index's name hasn't change fails to get noticed for an
> alarmingly long period of time after the change has happened. I also
> suspect that an appropriate fix might be to ensure that
> AcceptInvalidationMessages() is run at least once at the beginning of
> parse analysis.

Why don't we just do that? I have run some performance tests and there
was no impact (when no invalidation events are generated). The code
path in the case where there are no events queued up looks pretty
harmless, certainly compared to all the other stuff that goes on per
command.

Then again, I don't know why you consider this such a big problem in the
first place. If a concurrent transaction doesn't get the new index name
until the next transaction, what's the problem?

Then again, I don't know why we don't just call
AcceptInvalidationMessages() before each command or at least before each
top-level command? That would make way too much sense.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2018-07-31 21:11:02 Re: Online enabling of checksums
Previous Message Bruce Momjian 2018-07-31 20:43:16 Re: Online enabling of checksums