From: Andrey Klychkov <aaklychkov@mail.ru>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Victor Yegorov <vyegorov@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re[2]: Alter index rename concurrently to
Date: Wed, 03 Oct 2018 14:51:29 +0300
Message-ID: <1538567488.953452852@f403.i.mail.ru> (raw)
In-Reply-To: <8cba25b5-e7c7-de36-7886-5488b37ec521@2ndquadrant.com>
References: <1531767486.432607658@f357.i.mail.ru>
<c3d9a44c-2ca1-6726-ad03-ed9c37cc7c29@2ndquadrant.com>
<8cba25b5-e7c7-de36-7886-5488b37ec521@2ndquadrant.com>
> Based on these assertions, here is my proposed patch. It lowers the
> lock level for index renaming to ShareUpdateExclusiveLock.
Hi, Peter
I made small review for your patch:
Server source code got from https://github.com/postgres/postgres.git
1. Patch was applied without any errors except a part related to documentation:
error: patch failed: doc/src/sgml/ref/alter_index.sgml:50
error: doc/src/sgml/ref/alter_index.sgml: patch does not apply
2. The code has been compiled successfully, configured by:
# ./configure CFLAGS="-O0" --enable-debug --enable-cassert --enable-depend --without-zlib
3. 'make' / 'make install' successfully made and complete.
4. The compiled instance has been started without any errors.
5. I realized several tests by the pgbench (with -c 4 -j 4 -T 360) that modified data into columns, indexed by pk and common btree.
In the same time there was a running script that was making renaming indexes multiple times in transactions with pg_sleep(1).
After several tests no errors were found.
6. pg_upgrade from 10 to 12 (patched) has been done without any errors / warnings
7. Code style:
+RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bool is_index)
This line is longer than 80 chars.
Thank you
>Вторник, 14 августа 2018, 9:33 +03:00 от Peter Eisentraut <peter.eisentraut@2ndquadrant.com>:
>
>On 31/07/2018 23:10, Peter Eisentraut wrote:
>> 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.
>
>Based on these assertions, here is my proposed patch. It lowers the
>lock level for index renaming to ShareUpdateExclusiveLock.
>
>--
>Peter Eisentraut http://www.2ndQuadrant.com/
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Regards,
Andrey Klychkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: aaklychkov@mail.ru, peter.eisentraut@2ndquadrant.com, robertmhaas@gmail.com, vyegorov@gmail.com
Subject: Re: Re[2]: Alter index rename concurrently to
In-Reply-To: <1538567488.953452852@f403.i.mail.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox