agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Fix drop replication slot blocking instead of returning error
37+ messages / 8 participants
[nested] [flat]
* [PATCH] Fix drop replication slot blocking instead of returning error
@ 2017-08-24 13:38 Simone Gotti <[email protected]>
2017-08-26 08:07 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Noah Misch <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
0 siblings, 2 replies; 37+ messages in thread
From: Simone Gotti @ 2017-08-24 13:38 UTC (permalink / raw)
To: pgsql-hackers
Hi all,
I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
active slot will block until it's released instead of returning an error
like
done in pg 9.6. Since this is a change in the previous behavior and the docs
wasn't changed I made a patch to restore the previous behavior.
Thanks,
Simone.
--
after git commit 9915de6c1cb calls to pg_drop_replication_slot or the
replication protocol DROP_REPLICATION_SLOT command will block when a
slot is in use instead of returning an error as before (as the doc
states).
This patch will set nowait to true to restore the previous
behavior.
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/x-patch] fix_drop_replication_slot_blocking.patch (910B, ../../CAEvsy6Wgdf90O6pUvg2wSVXL2omH5OPC-38OD4Zzgk-FXavj3Q@mail.gmail.com/3-fix_drop_replication_slot_blocking.patch)
download | inline diff:
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index d4cbd83bde..ab776e85d2 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -171,7 +171,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
CheckSlotRequirements();
- ReplicationSlotDrop(NameStr(*name), false);
+ ReplicationSlotDrop(NameStr(*name), true);
PG_RETURN_VOID();
}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 03e1cf44de..c6b40ec0fb 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1028,7 +1028,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
static void
DropReplicationSlot(DropReplicationSlotCmd *cmd)
{
- ReplicationSlotDrop(cmd->slotname, false);
+ ReplicationSlotDrop(cmd->slotname, true);
EndCommand("DROP_REPLICATION_SLOT", DestRemote);
}
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
@ 2017-08-26 08:07 ` Noah Misch <[email protected]>
2017-08-29 10:04 ` Re: Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
1 sibling, 1 reply; 37+ messages in thread
From: Noah Misch @ 2017-08-26 08:07 UTC (permalink / raw)
To: [email protected]; +Cc: Simone Gotti <[email protected]>; pgsql-hackers
On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote:
> I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> active slot will block until it's released instead of returning an error
> like
> done in pg 9.6. Since this is a change in the previous behavior and the docs
> wasn't changed I made a patch to restore the previous behavior.
>
> Thanks,
>
> Simone.
>
> --
>
> after git commit 9915de6c1cb calls to pg_drop_replication_slot or the
> replication protocol DROP_REPLICATION_SLOT command will block when a
> slot is in use instead of returning an error as before (as the doc
> states).
>
> This patch will set nowait to true to restore the previous
> behavior.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Álvaro,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-26 08:07 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Noah Misch <[email protected]>
@ 2017-08-29 10:04 ` Alvaro Herrera <[email protected]>
2017-09-01 02:14 ` Re: Re: [PATCH] Fix drop replication slot blocking instead of returning error Noah Misch <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Alvaro Herrera @ 2017-08-29 10:04 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Simone Gotti <[email protected]>; pgsql-hackers
Noah Misch wrote:
> On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote:
> > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> > active slot will block until it's released instead of returning an error
> > like
> > done in pg 9.6. Since this is a change in the previous behavior and the docs
> > wasn't changed I made a patch to restore the previous behavior.
> > after git commit 9915de6c1cb calls to pg_drop_replication_slot or the
> > replication protocol DROP_REPLICATION_SLOT command will block when a
> > slot is in use instead of returning an error as before (as the doc
> > states).
> >
> > This patch will set nowait to true to restore the previous
> > behavior.
> The above-described topic is currently a PostgreSQL 10 open item.
Looking at it now -- next report tomorrow.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-26 08:07 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Noah Misch <[email protected]>
2017-08-29 10:04 ` Re: Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
@ 2017-09-01 02:14 ` Noah Misch <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Noah Misch @ 2017-09-01 02:14 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Simone Gotti <[email protected]>; pgsql-hackers
On Tue, Aug 29, 2017 at 12:04:33PM +0200, Alvaro Herrera wrote:
> Noah Misch wrote:
> > On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote:
> > > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> > > active slot will block until it's released instead of returning an error
> > > like
> > > done in pg 9.6. Since this is a change in the previous behavior and the docs
> > > wasn't changed I made a patch to restore the previous behavior.
>
> > > after git commit 9915de6c1cb calls to pg_drop_replication_slot or the
> > > replication protocol DROP_REPLICATION_SLOT command will block when a
> > > slot is in use instead of returning an error as before (as the doc
> > > states).
> > >
> > > This patch will set nowait to true to restore the previous
> > > behavior.
>
> > The above-described topic is currently a PostgreSQL 10 open item.
>
> Looking at it now -- next report tomorrow.
This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
@ 2017-08-29 10:13 ` Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
1 sibling, 1 reply; 37+ messages in thread
From: Alvaro Herrera @ 2017-08-29 10:13 UTC (permalink / raw)
To: Simone Gotti <[email protected]>; +Cc: pgsql-hackers
Simone Gotti wrote:
> Hi all,
>
> I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> active slot will block until it's released instead of returning an error
> like
> done in pg 9.6. Since this is a change in the previous behavior and the docs
> wasn't changed I made a patch to restore the previous behavior.
Changing that behavior was the entire point of the cited commit.
A better fix, from my perspective, is to amend the docs as per the
attached patch. This is what would be useful for logical replication,
which is what replication slots were invented for in the first place.
If you disagree, let's discuss what other use cases you have, and we can
come up with alternatives that satisfy both. I think a decent answer,
but one which would create a bit of extra churn, would be to have an
optional boolean flag in the command/function for "nowait", instead of
hardcoding either behavior.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
@ 2017-08-29 11:42 ` Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Simone Gotti @ 2017-08-29 11:42 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: pgsql-hackers
On Tue, Aug 29, 2017 at 12:13 PM, Alvaro Herrera
<[email protected]> wrote:
>
Hi Alvaro,
> Simone Gotti wrote:
> > Hi all,
> >
> > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> > active slot will block until it's released instead of returning an error
> > like
> > done in pg 9.6. Since this is a change in the previous behavior and the docs
> > wasn't changed I made a patch to restore the previous behavior.
>
> Changing that behavior was the entire point of the cited commit.
Sorry, I was thinking that the new behavior was needed for internal
future functions since the doc wasn't changed.
>
> A better fix, from my perspective, is to amend the docs as per the
> attached patch. This is what would be useful for logical replication,
> which is what replication slots were invented for in the first place.
I don't know the reasons why the new behavior is better for logical
replication so I trust you. We are using repl slots for physical
replication.
> If you disagree, let's discuss what other use cases you have, and we can
> come up with alternatives that satisfy both.
I just faced the opposite problem, in stolon [1], we currently rely on
the previous behavior. i.e. we don't want to block waiting for a slot
to be released (that under some partitioning conditions could not
happen for a long time), but prefer to continue retrying the drop
later. Now we partially avoid blocking timing out the drop operation
after some seconds.
Another idea will be to query the slot status before doing the drop
but will lead to a race condition (probably the opposite that that
commit was fixing) if the slot is acquired between the query and the
drop.
> I think a decent answer,
> but one which would create a bit of extra churn, would be to have an
> optional boolean flag in the command/function for "nowait", instead of
> hardcoding either behavior.
I think that this will be the best fix. I'm not sure on the policy of
these commands and if backward compatibility will be better (in such
case the old behavior should be the default and a new "wait" flag
could be added).
If the default behavior is going to change we have to add different
code for postgres >= 10.
Thanks,
Simone.
[1] https://github.com/sorintlab/stolon
>
>
> --
> Álvaro Herrera https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
@ 2017-08-29 14:02 ` Andres Freund <[email protected]>
2017-08-30 00:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Craig Ringer <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Andres Freund @ 2017-08-29 14:02 UTC (permalink / raw)
To: Simone Gotti <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers
Hi,
On 2017-08-29 13:42:05 +0200, Simone Gotti wrote:
> On Tue, Aug 29, 2017 at 12:13 PM, Alvaro Herrera
> <[email protected]> wrote:
> >
>
> Hi Alvaro,
>
> > Simone Gotti wrote:
> > > Hi all,
> > >
> > > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an
> > > active slot will block until it's released instead of returning an error
> > > like
> > > done in pg 9.6. Since this is a change in the previous behavior and the docs
> > > wasn't changed I made a patch to restore the previous behavior.
> >
> > Changing that behavior was the entire point of the cited commit.
>
> Sorry, I was thinking that the new behavior was needed for internal
> future functions since the doc wasn't changed.
FWIW, I also don't think it's ok to just change the behaviour
unconditionally and without a replacement for existing behaviour.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
@ 2017-08-30 00:36 ` Craig Ringer <[email protected]>
2017-08-30 13:33 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Craig Ringer @ 2017-08-30 00:36 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Simone Gotti <[email protected]>; Alvaro Herrera <[email protected]>; pgsql-hackers
On 29 August 2017 at 22:02, Andres Freund <[email protected]> wrote:
> Hi,
>
> On 2017-08-29 13:42:05 +0200, Simone Gotti wrote:
> > On Tue, Aug 29, 2017 at 12:13 PM, Alvaro Herrera
> > <[email protected]> wrote:
> > >
> >
> > Hi Alvaro,
> >
> > > Simone Gotti wrote:
> > > > Hi all,
> > > >
> > > > I noticed that in postgres 10beta3, calling pg_drop_replication_slot
> on an
> > > > active slot will block until it's released instead of returning an
> error
> > > > like
> > > > done in pg 9.6. Since this is a change in the previous behavior and
> the docs
> > > > wasn't changed I made a patch to restore the previous behavior.
> > >
> > > Changing that behavior was the entire point of the cited commit.
> >
> > Sorry, I was thinking that the new behavior was needed for internal
> > future functions since the doc wasn't changed.
>
> FWIW, I also don't think it's ok to just change the behaviour
> unconditionally and without a replacement for existing behaviour.
Seems like it just needs a new argument nowait DEFAULT false
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
2017-08-30 00:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Craig Ringer <[email protected]>
@ 2017-08-30 13:33 ` Alvaro Herrera <[email protected]>
2017-08-30 13:39 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Alvaro Herrera @ 2017-08-30 13:33 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Andres Freund <[email protected]>; Simone Gotti <[email protected]>; pgsql-hackers
Craig Ringer wrote:
> > FWIW, I also don't think it's ok to just change the behaviour
> > unconditionally and without a replacement for existing behaviour.
>
> Seems like it just needs a new argument nowait DEFAULT false
I added a WAIT flag to DROP_REPLICATION_SLOT, preliminary patch
attached. Running tests now.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
2017-08-30 00:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Craig Ringer <[email protected]>
2017-08-30 13:33 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
@ 2017-08-30 13:39 ` Alvaro Herrera <[email protected]>
2017-09-01 14:31 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Alvaro Herrera @ 2017-08-30 13:39 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Andres Freund <[email protected]>; Simone Gotti <[email protected]>; pgsql-hackers
And another patch to restore behavior to replication origin drop.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
2017-08-30 00:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Craig Ringer <[email protected]>
2017-08-30 13:33 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-30 13:39 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
@ 2017-09-01 14:31 ` Alvaro Herrera <[email protected]>
2017-09-01 14:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Alvaro Herrera @ 2017-09-01 14:31 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Andres Freund <[email protected]>; Simone Gotti <[email protected]>; pgsql-hackers
Both patches pushed, which should close the open item.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: [PATCH] Fix drop replication slot blocking instead of returning error
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 10:13 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-29 14:02 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Andres Freund <[email protected]>
2017-08-30 00:36 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Craig Ringer <[email protected]>
2017-08-30 13:33 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-08-30 13:39 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
2017-09-01 14:31 ` Re: [PATCH] Fix drop replication slot blocking instead of returning error Alvaro Herrera <[email protected]>
@ 2017-09-01 14:36 ` Simone Gotti <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Simone Gotti @ 2017-09-01 14:36 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Craig Ringer <[email protected]>; Andres Freund <[email protected]>; pgsql-hackers
On Fri, Sep 1, 2017 at 4:31 PM, Alvaro Herrera <[email protected]> wrote:
>
> Both patches pushed, which should close the open item.
Hi Alvaro,
thanks a lot!
Cheers,
Simone.
>
> --
> Álvaro Herrera https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH 2/2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b1ce83e1dd..c5f257ce15 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -122,6 +122,7 @@ typedef struct ReindexIndexInfo
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
/*
@@ -395,7 +396,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1574,9 +1575,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only takes a snapshot to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3343,6 +3346,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3384,6 +3390,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3451,6 +3458,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3480,6 +3492,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3500,8 +3516,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3530,6 +3552,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3564,6 +3590,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* To ensure no deadlocks, we must commit and start yet another
* transaction, and do our wait before any snapshot has been taken in
* it.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
CommitTransactionCommand();
StartTransactionCommand();
@@ -3594,6 +3623,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3641,6 +3677,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only takes a snapshot to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3671,6 +3713,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only takes a snapshot to do some catalog manipulations, after all
+ * the waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index e77f76ae8a..e1a6bc5170 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--GvXjxJ+pjyke8COw--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
@ 2020-11-30 19:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Alvaro Herrera @ 2020-11-30 19:01 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8c9c39a467..35c3d20eae 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations
- * later.) Processes running CREATE INDEX CONCURRENTLY
+ * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */
- if (safe_index)
- set_indexsafe_procflags();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ /* determine safety of this index for set_indexsafe_procflags */
+ idx->safe = (indexRel->rd_indexprs == NIL &&
+ indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 2 of REINDEX CONCURRENTLY
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
StartTransactionCommand();
+ /*
+ * Because we don't take a snapshot in this transaction, there's no need
+ * to set the PROC_IN_SAFE_IC flag here.
+ */
+
/*
* Phase 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
CHECK_FOR_INTERRUPTS();
+ /* Tell concurrent indexing to ignore us, if index qualifies */
+ if (newidx->safe)
+ set_indexsafe_procflags();
+
/*
* Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *
+ * Because we don't take a snapshot in this transaction, there's no
+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
+ /*
+ * Because this transaction only does catalog manipulations and doesn't do
+ * any index operations, we can set the PROC_IN_SAFE_IC flag here
+ * unconditionally.
+ */
+ set_indexsafe_procflags();
+
forboth(lc, indexIds, lc2, newIndexIds)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after the
+ * wait is over.
+ */
+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*
+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because
+ * it only acquires an Xid to do some catalog manipulations, after all the
+ * waiting has been completed.
+ */
+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0786fcf103..683ab64f76 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
--tKW2IUtsqtDRztdT--
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Enhance file_fdw to report processed and skipped tuples in COPY progress
@ 2025-03-05 16:13 Fujii Masao <[email protected]>
2025-03-09 11:38 ` Re: Enhance file_fdw to report processed and skipped tuples in COPY progress vignesh C <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: Fujii Masao @ 2025-03-05 16:13 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; pgsql-hackers
On 2024/11/30 15:23, Kirill Reshke wrote:
> On Fri, 11 Oct 2024 at 06:53, Fujii Masao <[email protected]> wrote:
>> However, this issue already exists without the proposed patch.
>> Since file_fdw already reports progress partially, querying multiple
>> file_fdw tables can lead to inaccurate or confusing progress reports.
>> You can even observe this when analyzing a file_fdw table and also
>> when copying to the table with a trigger that executes progress-reporting
>> commands.
>>
>> So, I don’t think this issue should block the proposed patch.
>> In fact, progress reporting is already flawed in these scenarios,
>> regardless of whether the patch is applied.
On second thought, supporting progress tracking for COPY used by file_fdw
could increase the chances of multiple commands being tracked simultaneously
by a single backend. This means the command progress view might show
incorrect results more often.
As I mentioned before, this issue already exists, but it currently
only happens in rare cases. I don’t think the fact that the issue
already exists is a good reason to introduce more, and likely more common,
scenarios where it could occur.
With that in mind, I'm thinking of withdrawing this patch for now.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Enhance file_fdw to report processed and skipped tuples in COPY progress
2025-03-05 16:13 Re: Enhance file_fdw to report processed and skipped tuples in COPY progress Fujii Masao <[email protected]>
@ 2025-03-09 11:38 ` vignesh C <[email protected]>
2025-03-09 23:47 ` Re: Enhance file_fdw to report processed and skipped tuples in COPY progress Fujii Masao <[email protected]>
0 siblings, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-03-09 11:38 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Masahiko Sawada <[email protected]>; pgsql-hackers
On Wed, 5 Mar 2025 at 21:43, Fujii Masao <[email protected]> wrote:
>
>
>
> On 2024/11/30 15:23, Kirill Reshke wrote:
> > On Fri, 11 Oct 2024 at 06:53, Fujii Masao <[email protected]> wrote:
> >> However, this issue already exists without the proposed patch.
> >> Since file_fdw already reports progress partially, querying multiple
> >> file_fdw tables can lead to inaccurate or confusing progress reports.
> >> You can even observe this when analyzing a file_fdw table and also
> >> when copying to the table with a trigger that executes progress-reporting
> >> commands.
> >>
> >> So, I don’t think this issue should block the proposed patch.
> >> In fact, progress reporting is already flawed in these scenarios,
> >> regardless of whether the patch is applied.
>
> On second thought, supporting progress tracking for COPY used by file_fdw
> could increase the chances of multiple commands being tracked simultaneously
> by a single backend. This means the command progress view might show
> incorrect results more often.
>
> As I mentioned before, this issue already exists, but it currently
> only happens in rare cases. I don’t think the fact that the issue
> already exists is a good reason to introduce more, and likely more common,
> scenarios where it could occur.
>
> With that in mind, I'm thinking of withdrawing this patch for now.
I've updated the status to "withdrawn." Feel free to add it again
anytime if you change your mind.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: Enhance file_fdw to report processed and skipped tuples in COPY progress
2025-03-05 16:13 Re: Enhance file_fdw to report processed and skipped tuples in COPY progress Fujii Masao <[email protected]>
2025-03-09 11:38 ` Re: Enhance file_fdw to report processed and skipped tuples in COPY progress vignesh C <[email protected]>
@ 2025-03-09 23:47 ` Fujii Masao <[email protected]>
0 siblings, 0 replies; 37+ messages in thread
From: Fujii Masao @ 2025-03-09 23:47 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Masahiko Sawada <[email protected]>; pgsql-hackers
On 2025/03/09 20:38, vignesh C wrote:
> I've updated the status to "withdrawn." Feel free to add it again
> anytime if you change your mind.
Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 37+ messages in thread
end of thread, other threads:[~2025-03-09 23:47 UTC | newest]
Thread overview: 37+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-08-24 13:38 [PATCH] Fix drop replication slot blocking instead of returning error Simone Gotti <[email protected]>
2017-08-26 08:07 ` Noah Misch <[email protected]>
2017-08-29 10:04 ` Alvaro Herrera <[email protected]>
2017-09-01 02:14 ` Noah Misch <[email protected]>
2017-08-29 10:13 ` Alvaro Herrera <[email protected]>
2017-08-29 11:42 ` Simone Gotti <[email protected]>
2017-08-29 14:02 ` Andres Freund <[email protected]>
2017-08-30 00:36 ` Craig Ringer <[email protected]>
2017-08-30 13:33 ` Alvaro Herrera <[email protected]>
2017-08-30 13:39 ` Alvaro Herrera <[email protected]>
2017-09-01 14:31 ` Alvaro Herrera <[email protected]>
2017-09-01 14:36 ` Simone Gotti <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH 2/2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2020-11-30 19:01 [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY Alvaro Herrera <[email protected]>
2025-03-05 16:13 Re: Enhance file_fdw to report processed and skipped tuples in COPY progress Fujii Masao <[email protected]>
2025-03-09 11:38 ` Re: Enhance file_fdw to report processed and skipped tuples in COPY progress vignesh C <[email protected]>
2025-03-09 23:47 ` Re: Enhance file_fdw to report processed and skipped tuples in COPY progress Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox