From: Noah Misch <noah@leadboat.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
Date: Mon, 18 Oct 2021 20:02:12 -0700
Message-ID: <20211019030212.GB3403374@rfd.leadboat.com> (raw)
In-Reply-To: <9BA43784-E4A7-4638-900C-FC9868C388FA@yandex-team.ru>
References: <20210815134547.3okuz6ktvu4qf7ig@alap3.anarazel.de>
<15936F8A-0436-40C8-9D1A-4A4623681554@yandex-team.ru>
<20210829180913.ghojzzsgkqgm7vki@alap3.anarazel.de>
<364BAA98-1A3F-4C39-B0D9-ED8C40EA902D@yandex-team.ru>
<20210908064515.GA2496820@rfd.leadboat.com>
<20210920044143.GA3414844@rfd.leadboat.com>
<593C6C66-F19E-412A-9896-F1F5B272424A@yandex-team.ru>
<20210925201012.GC4134968@rfd.leadboat.com>
<20211017151205.GA2384251@rfd.leadboat.com>
<9BA43784-E4A7-4638-900C-FC9868C388FA@yandex-team.ru>
On Mon, Oct 18, 2021 at 06:23:05PM +0500, Andrey Borodin wrote:
> > 17 окт. 2021 г., в 20:12, Noah Misch <noah@leadboat.com> написал(а):
> > I think the attached version is ready for commit. Notable differences
> > vs. v14:
> Few notes:
>
> 1. Maybe an Assert(in_progress_list_maxlen) when in_progress_list_maxlen is used?
RelationCacheInitialize() initializes both in_progress_list_maxlen and the
RelationIdCache hash table, and any ERROR there is promoted to FATAL. Without
the hash table, nothing good can happen in relcache. Hence, I think such an
assert is excessive.
> 2.
> -#define VirtualTransactionIdIsPreparedXact(vxid) \
> +#define VirtualTransactionIdIsRecoveredPreparedXact(vxid) \
>
> This is a very neat transition. Yes, the function argument will always be a xid only for recovered transactions. Maybe add a comment here that this function is expected to be used only for results of GetLockConflicts()?
One can use it on any VirtualTransactionId, though some sources only return
values for which this returns false. It can return true for the result of
GET_VXID_FROM_PGPROC(). I think it can return true for the result of
GetCurrentVirtualXIDs(limitXmin = InvalidTransactionId), but core code doesn't
make such a call.
> > One thing not done here is to change the tests to use CREATE INDEX
> > CONCURRENTLY instead of REINDEX CONCURRENTLY, so they're back-patchable to v11
> > and earlier. I may do that before pushing, or I may just omit the tests from
> > older branches.
>
> The tests refactors PostgresNode.pm and some tests. Back-patching this would be quite invasive.
That's fine with me. Back-patching a fix without its tests is riskier than
back-patching test infrastructure changes.
> But swapping every "REINDEX INDEX CONCURRENTLY idx;" with
> DROP INDEX CONCURRENTLY idx;
> CREATE INDEX CONCURRENTLY idx on tbl(i);
> works.
>
> > <inval-build-race-v1.patch><prepared-transactions-cic-series202107-v15nm.patch>
> I've checked that this patches work for some time on my machines. I do not observe failures.
Good. Thanks for checking.
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-bugs@postgresql.org
Cc: noah@leadboat.com, x4mmm@yandex-team.ru, pgsql-bugs@lists.postgresql.org, michael@paquier.xyz, tgl@sss.pgh.pa.us, pg@bowt.ie, andres@anarazel.de
Subject: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
In-Reply-To: <20211019030212.GB3403374@rfd.leadboat.com>
* 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