From: Álvaro Herrera <alvherre@alvh.no-ip.org>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Hamid Akhtar <hamid.akhtar@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements
Date: Tue, 23 Feb 2021 12:26:23 -0300
Message-ID: <20210223152623.GA2638@alvherre.pgsql> (raw)
In-Reply-To: <CAD21AoC3xp9-xrM+HFB1qKe9aX0-arvYS8M8KfJYPjJRDJn+-w@mail.gmail.com>
On 2021-Feb-23, Masahiko Sawada wrote:
> I've looked at the v3 patch and it looks good to me. A minor comment is:
>
> + /* Catalog tables need to consider all backends. */
> + h->catalog_oldest_nonremovable =
> + TransactionIdOlder(h->catalog_oldest_nonremovable, xmin);
> +
>
> I think that the above comment is not accurate since we consider only
> backends on the same database in some cases.
You're right. Adjusted.
> the patch makes the following change:
>
> @@ -1847,7 +1871,6 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
> h->shared_oldest_nonremovable =
> TransactionIdOlder(h->shared_oldest_nonremovable,
> h->slot_catalog_xmin);
> - h->catalog_oldest_nonremovable = h->data_oldest_nonremovable;
> h->catalog_oldest_nonremovable =
> TransactionIdOlder(h->catalog_oldest_nonremovable,
> h->slot_catalog_xmin);
>
> In the original code, catalog_oldest_nonremovable is initialized with
> data_oldest_nonremovable that considered slot_xmin. Therefore, IIUC
> catalog_oldest_nonremovable is the oldest transaction id among
> data_oldest_nonremovable, slot_xmin, and slot_catalog_xmin. But with
> the patch, catalog_oldest_nonremovable doesn't consider slot_xmin. It
> seems okay to me as catalog_oldest_nonremovable is interested in only
> slot_catalog_xmin.
I tend to agree -- it seems safe to ignore slot_xmin in that case.
However, tracing through the maze of xmin vs. catalog_xmin transmission
across the whole system is not trivial. I changed the patch so that we
adjust to slot_xmin first, then to slot_catalog_xmin. I *think* the end
effect should be the same. But if we want to make that claim to save
that one assignment, we can make it in a separate commit. However,
given that this comparison and assignment is outside the locked area, I
don't think it's worth bothering with.
Thanks for the input! I have pushed this patch.
--
Álvaro Herrera Valdivia, Chile
"El sentido de las cosas no viene de las cosas, sino de
las inteligencias que las aplican a sus problemas diarios
en busca del progreso." (Ernesto Hernández-Novich)
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: alvherre@alvh.no-ip.org, sawada.mshk@gmail.com, boekewurm+postgres@gmail.com, hamid.akhtar@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements
In-Reply-To: <20210223152623.GA2638@alvherre.pgsql>
* 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