From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Garrett Thornburg <film42(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: PATCH: Add REINDEX tag to event triggers |
Date: | 2023-10-28 04:15:22 |
Message-ID: | CACJufxGjd+Za5wnvRnnFXnPz=mtc2ZU_pRknoQKxvCAisFMMsA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Oct 27, 2023 at 3:15 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Sep 04, 2023 at 08:00:52PM +0200, Jim Jones wrote:
> > LGTM. It applies and builds cleanly, all tests pass and documentation also
> > builds ok. The CFbot seems also much happier now :)
>
> + /*
> + * Open and lock the relation. ShareLock is sufficient since we only need
> + * to prevent schema and data changes in it. The lock level used here
> + * should match catalog's reindex_relation().
> + */
> + rel = try_table_open(relid, ShareLock);
>
> I was eyeing at 0003, and this strikes me as incorrect. Sure, this
> matches what reindex_relation() does, but you've missed that
> CONCURRENTLY takes a lighter ShareUpdateExclusiveLock, and ShareLock
> conflicts with it. See:
> https://www.postgresql.org/docs/devel/explicit-locking.html
>
> So, doesn't this disrupt a concurrent REINDEX?
> --
> Michael
ReindexPartitions called ReindexMultipleInternal
ReindexRelationConcurrently add reindex_event_trigger_collect to cover
it. (line 3869)
ReindexIndex has the function reindex_event_trigger_collect. (line 2853)
reindex_event_trigger_collect_relation called in
ReindexMultipleInternal, ReindexTable (line 2979).
Both are "under concurrent is false" branches.
So it should be fine.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-10-28 04:28:53 | Re: Synchronizing slots from primary to standby |
Previous Message | Tom Lane | 2023-10-28 01:44:30 | Re: maybe a type_sanity. sql bug |