| From: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, ashutosh(dot)bapat(dot)oss(at)gmail(dot)com |
| Subject: | Re: Crashes on a partition whose concurrent detach never finished |
| Date: | 2026-09-10 11:51:01 |
| Message-ID: | CABdArM4fJ8JAWyP0MAF=8n66M6=CouUmtSGwqizn2pD=gfPEZw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Sep 8, 2026 at 12:35 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Sep 8, 2026 at 10:42 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> >
> > I tested it further on my machine. Here are the observations:
> >
> >
> > 2)
> > FOR TABLE root pub:
> > During the intermediate state of DETACH PARTITION ... CONCURRENTLY,
> > changes made directly to the partition being detached (t1_part1) are
> > also replicated, even though t1_part1 is not explicitly listed in
> > pg_publication_tables and pg_partition_root(t1_part1) already returns
> > t1_part1. This needs some thought regarding how it should behave.
> > Should t1_part1 not replicated here? Thoughts?
> >
> > Once the detach completes, changes to t1_part1 are no longer
> > replicated through the publication of t1, which appears correct.
>
> On debugging further, t1_part1 is also replicated (incorrectly IMO) in
> Case 2 because pgoutput's RelationSyncCache is not invalidated during
> the first phase of DETACH PARTITION CONCURRENTLY.
>
> In ATExecDetachPartition(), MarkInheritDetached() only updates the
> partition's pg_inherits row (inhdetachpending = true), without
> generating a cache invalidation. Before commit, only the parent's
> relcache is explicitly invalidated via CacheInvalidateRelcache(rel) in
> ATExecDetachPartition().
>
> As a result, the partition's previously cached (pre-detach)
> RelationSyncEntry is reused, so changes to it continue to be
> replicated. This is also confirmed by the fact that if no INSERT is
> executed on t1_part1 before DETACH CONCURRENTLY blocks, replication
> behaves correctly: the partition is not replicated because a new
> RelationSyncCache entry is correctly built after inhdetachpending is
> set.
>
> Once FINALIZE runs, the issue self-corrects because
> DetachPartitionFinalize() updates the partition's pg_class row
> (relispartition = false), which invalidates its relcache, and also
> DetachPartitionFinalize() explicitly invalidates the parent and
> descendants.
>
> Adding cache-invalidation for part-table here solves the problem, but
> I am not sure if it could have any other side-effects. Please have a
> look.
>
> @@ -21758,6 +21758,8 @@ ATExecDetachPartition(List **wqueue,
> AlteredTableInfo *tab, Relation rel,
> /* Invalidate relcache entries for the parent -- must
> be before close */
> CacheInvalidateRelcache(rel);
>
> + CacheInvalidateRelcache(partRel);
> +
> table_close(partRel, NoLock);
> table_close(rel, NoLock);
> tab->rel = NULL;
>
+1.
Without invalidation, the publication behavior can become nondeterministic.
We also need to invalidate the whole partRel subtree (if exists), as
we do in FINALIZE phase-2. For members of this subtree, the
publication decision should now be based on partRel instead of rel;
otherwise, we can see the same nondeterministic behavior for them.
I also looked at an alternative that avoids this invalidation: follow
the former root until FINALIZE, i.e. publish based on the root's
publication status until the detach completes.
The problem with this approach is that it can cause publisher and
subscriber data to diverge when publish_via_partition_root = true.
For example, suppose the publisher has t1 with partitions p1 and p2,
while the subscriber has only t1. If t1.p1 is in detach pending and we
continue publishing it, the subscriber will apply its changes to t1,
while the publisher has already stopped showing the new rows from p1
in SELECT * FROM t1.
The changes from the detached partition are published using the
parent's identity, so the subscriber applies them to its own parent.
But the publisher's parent no longer includes those rows because the
partition is no longer in its partition descriptor. As a result,
SELECT * FROM parent can return different data on the publisher and
subscriber. This difference can continue to grow for as long as the
partition remains in detach pending.
So this approach does not seem feasible.
~~~
Given that we are treating a detach-pending partition as an individual
table for publication decisions, the changes in patch-003 (pg19
regression) look correct to me. The changes in relcache.c correctly
treat the detach-pending partition as an individual table.
--
Thanks,
Nisha
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-09-10 12:06:58 | Re: Crashes on a partition whose concurrent detach never finished |
| Previous Message | Shinya Kato | 2026-09-10 11:47:04 | Re: pgbench: \gset and \aset should store SQL NULL as the null value |