On Wed, Aug 12, 2020 at 10:37:08PM +0900, Michael Paquier wrote:
> I have been able to work more on this patch today, and finally added
> an error context for the transaction block check, as that's cleaner.
> In my manual tests, I have also bumped into a case that failed with
> the original patch (where there were no session locks), and created
> an isolation test based on it: drop of a partition leaf concurrently
> to REINDEX done on the parent. One last thing I have spotted is that
> we failed to discard properly foreign tables defined as leaves of a
> partition tree, causing a reindex to fail, so reindex_partitions()
> ought to just use RELKIND_HAS_STORAGE() to do its filtering work. I
> am leaving this patch alone for a couple of days now, and I'll try to
> come back to it after and potentially commit it. The attached has
> been indented by the way.
I got to think more about the locking strategy used in this patch, and
I am afraid that we should fix the bug with REINDEX SCHEMA/DATABASE
first. What we have here is rather similar to a REINDEX SCHEMA with
all the partitions on the same schema, so it would be better to apply
the same set of assumptions and logic for the reindex of partitioned
relations as we do for the others. This makes the whole logic more
consistent, but it also reduces the surface of bugs. I have created a
separate thread for the problem, and posted a patch:
https://www.postgresql.org/message-id/20200813043805.GE11663@paquier.xyz
Once this gets done, we should then be able to get rid of the extra
session locking taken when building the list of partitions, limiting
session locks to only be taken during the concurrent reindex of a
single partition (the table itself for a partition table, and the
parent table for a partition index), making the whole operation less
invasive.
--
Michael
