BUG #18644: ALTER PUBLICATION ... SET (publish_via_partition_root) wrong/undocumented behavior.

Dear Maxim,

Thanks for reporting the issue. Before discussing more about it, I want to confirm
the exact workload you did. Is below sequence same as you expected?

0. There is a table which have leaf tables on publisher/subscriber.
1. create a PUBLICATION and include a root table
2. create a SUBSCRIPTION to subscribe above
3. insert some data on pub and wait until the replication is done.
4. do ALTER PUBLICAITON WITH (publish_via_partition_root = true)
5. insert tuples on the publisher
-> tuples won't be replicated until ALTER SUBSCRIPTION REFRESH PUBLICATION is done.

If the copy_data is true for REFRESH, all tuples inserted at 3. will be copied again.
Otherwise, data between ALTER PUB. - ALTER SUB. will be lost.

Attached .pl file (renamed to .txt due to the security reason) can reproduce and
test the situation via our test framework.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, Oct 2, 2024 at 12:14 AM PG Bug reporting form
<noreply@postgresql.org> wrote:

The following bug has been logged on the website:

...

When ALTER PUBLICATION ... SET (publish_via_partition_root) executed on the
existing logical replication with data
(following ALTER SUBSCRIPTION ... REFRESH PUBLICATION), the database start
copy whole partitioned table from start (thus breaking existing logical
replication).
What's worse - I didn't found any way get out of such situation less than
redo all multi-TB logical replication from blank db.

Also ALTER SUBSCRIPTION ... REFRESH PUBLICATION (copy_data=false) - cannot
be used as workaround because it lead to loose any changes in partitioned
table between run ALTER PUBLICATION and ALTER SUBSCRIPTION.

Afterthought this behavior not surprising at all but I think better to
document it somewhere.

Can you tell us what additional information you want to document other
than what is already documented in CREATE PUBLICATION [1]https://www.postgresql.org/docs/devel/sql-createpublication.html -- With Regards, Amit Kapila. related to
this parameter?

It would be useful if you can create a small test case to show the
exact problem and what is your usecase for the same?

[1]: https://www.postgresql.org/docs/devel/sql-createpublication.html -- With Regards, Amit Kapila.
--
With Regards,
Amit Kapila.

On Fri, Oct 4, 2024 at 7:30 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Dear Maxim,

Thanks for reporting the issue. Before discussing more about it, I want to confirm
the exact workload you did. Is below sequence same as you expected?

Yes it is exactly the my case/sequence of actions (but with 10TB database ().
Thank you for creating a reproducer (I was working on it but due time
constraints I didn't finish it before you posted your test case).

--
Maxim Boguk
Senior Postgresql DBA

Phone UA: +380 99 143 0000
Phone AU: +61 45 218 5678

Can you tell us what additional information you want to document other
than what is already documented in CREATE PUBLICATION [1] related to
this parameter?

At least put a warning that alter publication set
(publish_via_partition_root) will unrecoverable break any existing
logical replication subscribers if there at least one partitioned
table with data in replication set,
so alter publication set (publish_via_partition_root) should be run
only before any subscription activation.
Ideally - make this use case actually working (but looking into the
code - it would be very complicated I feel).

It would be useful if you can create a small test case to show the exact problem and what is your usecase for the same?

Usecase - after initial load of logical replica I decided that on the
replica I better split future data into weekly partitions due huge
size (instead of monthly partitions on the master/publisher)
exactly the case for "alter publication set (publish_via_partition_root)".

My main issues with this case - there is no way to fix this problem if
it happened less than reloading whole logical replication from blank.

--
Maxim Boguk
Senior Postgresql DBA

Phone UA: +380 99 143 0000
Phone AU: +61 45 218 5678

On Fri, Oct 4, 2024 at 7:38 PM Maxim Boguk <maxim.boguk@gmail.com> wrote:

It would be useful if you can create a small test case to show the exact problem and what is your usecase for the same?

Usecase - after initial load of logical replica I decided that on the
replica I better split future data into weekly partitions due huge
size (instead of monthly partitions on the master/publisher)
exactly the case for "alter publication set (publish_via_partition_root)".

My main issues with this case - there is no way to fix this problem if
it happened less than reloading whole logical replication from blank.

You can prevent the problem by avoiding writes to the partitioned
tables between the Alter Pub and Alter Sub steps. One idea could be
that in a parallel session on publisher lock the parent table in
Access Exclusive mode till the Alter Sub command (with
copy_data=false) is finished.

--
With Regards,
Amit Kapila.

On Tue, Oct 8, 2024 at 12:22 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Oct 4, 2024 at 7:38 PM Maxim Boguk <maxim.boguk@gmail.com> wrote:

It would be useful if you can create a small test case to show the exact problem and what is your usecase for the same?

Usecase - after initial load of logical replica I decided that on the
replica I better split future data into weekly partitions due huge
size (instead of monthly partitions on the master/publisher)
exactly the case for "alter publication set (publish_via_partition_root)".

My main issues with this case - there is no way to fix this problem if
it happened less than reloading whole logical replication from blank.

You can prevent the problem by avoiding writes to the partitioned
tables between the Alter Pub and Alter Sub steps. One idea could be
that in a parallel session on publisher lock the parent table in
Access Exclusive mode till the Alter Sub command (with
copy_data=false) is finished.

--
With Regards,
Amit Kapila.

Thank you, it should be work. Unfortunately my English writing now is
not good enough to suggest correct and easy to understand warnings in
the documentation about this issue.

--
Maxim Boguk
Senior Postgresql DBA

Phone UA: +380 99 143 0000
Phone AU: +61 45 218 5678

Dear Maxim, Amit,

Thank you, it should be work. Unfortunately my English writing now is
not good enough to suggest correct and easy to understand warnings in
the documentation about this issue.

I've written a doc patch to add a caution in ALTER PUBLICATION page. How do you feel?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, 9 Oct 2024 at 11:01, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Dear Maxim, Amit,

Thank you, it should be work. Unfortunately my English writing now is
not good enough to suggest correct and easy to understand warnings in
the documentation about this issue.

I've written a doc patch to add a caution in ALTER PUBLICATION page. How do you feel?

Few comments:
1) How about changing:
+      <para>
+       Altering the <literal>publish_via_partition_root</literal> during the
+       replication can lead to data loss or duplication. This is because the
+       publisher replicates changes as the partitioned table after the
+       altering, but it is not listed in
+       <link linkend="catalog-pg-subscription-rel"><literal>pg_subscription_rel</literal></link>.
+      </para>

to:
When the partition root table is specified as the replication target
instead of its leaf tables, changing the publish_via_partition_root
parameter during replication can cause data loss or duplication. This
happens because the subscriber initially subscribed to the leaf
tables. After running the ALTER PUBLICATION ... SET and ALTER
SUBSCRIPTION ... REFRESH PUBLICATION command, the subscriber will
subscribe to the partition root table.

2) Should we change:
+       To prevent the issue, you can avoid modifying the partitioned table
+       between the <command>ALTER PUBLICATION ... SET</command> and
+       <link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... REFRESH PUBLICATION</command></link>.

to:
+       To prevent the issue, you can avoid modifying the partitioned table
+       between the <command>ALTER PUBLICATION ... SET</command> and
+       <link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... REFRESH PUBLICATION</command></link> and refresh
publication with copy_data as off.

Regards,
Vignesh

Dear Vignesh,

Thanks for giving comments! Since I'm not a native I followed your points.
I ran Grammaly just in case and it said OK.

PSA new version.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, Oct 9, 2024 at 6:04 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Thanks for giving comments! Since I'm not a native I followed your points.
I ran Grammaly just in case and it said OK.

PSA new version.

+     <caution>
+      <para>
+       When the partition root table is specified as the replication target
+       instead of its leaf tables, altering the
+       <literal>publish_via_partition_root</literal> parameter during
+       replication can cause data loss or duplication. This happens because
+       the subscriber initially subscribed to the leaf tables.

This assumes that the user is always changing
publish_via_partition_root from 'false' to 'true'. Can't she change
from 'false' to 'true' as well?

+      <para>
+       To prevent the issue, you can avoid modifying the partitioned table
+       between the <command>ALTER PUBLICATION ... SET</command> and

Can't the problem happen when any of the leaf tables are modified? If
so, that is not clear from the above statement.

--
With Regards,
Amit Kapila.

+      <para>
+       To prevent the issue, you can avoid modifying the partitioned table
+       between the <command>ALTER PUBLICATION ... SET</command> and

Can't the problem happen when any of the leaf tables are modified? If
so, that is not clear from the above statement.

Problem also happens if publication is created via FOR ALL TABLES /
FOR TABLES IN SCHEMA (without directly including partition head into
publication).

I have a suspicion that including partition head (directly or via FOR
ALL TABLES) into the initial publication - really required to get a
problem.
What is expected behavior in case when only partitions are included
into publication/subscription set, but partition head isn't included
and publish_via_partition_root='true' executed?

--
Maxim Boguk
Senior Postgresql DBA

Phone UA: +380 99 143 0000
Phone AU: +61 45 218 5678

Dear Amit,

Thanks for giving comments. PSA new version patch.

+     <caution>
+      <para>
+       When the partition root table is specified as the replication target
+       instead of its leaf tables, altering the
+       <literal>publish_via_partition_root</literal> parameter during
+       replication can cause data loss or duplication. This happens because
+       the subscriber initially subscribed to the leaf tables.

This assumes that the user is always changing
publish_via_partition_root from 'false' to 'true'. Can't she change
from 'false' to 'true' as well?

Yeah, users can change for arbitrary directions, and the issue can happen in any
cases. Toggling the option changes the oid contained in the replication messages,
this causes a loss. I adjusted a description.

+      <para>
+       To prevent the issue, you can avoid modifying the partitioned table
+       between the <command>ALTER PUBLICATION ... SET</command>
and

Can't the problem happen when any of the leaf tables are modified? If
so, that is not clear from the above statement.

Partition root table is a virtual object, and actual changes are done at its leaf.
Based on that "modifying the partitioned table" may be odd. How about "modifying
any leaf tables"?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Dear Maxim,

Problem also happens if publication is created via FOR ALL TABLES /
FOR TABLES IN SCHEMA (without directly including partition head into
publication).

Thanks for pointing out. I also confirmed this issue can happen for
FOR ALL TABLES/FOR TABLES IN SCHEMA publications. I modified a documentation.

What is expected behavior in case when only partitions are included
into publication/subscription set, but partition head isn't included
and publish_via_partition_root='true' executed?

Just to confirm - you meant like below definitions, right?

```
create table tab (a int) partition by range (a);
create table tab_1 partition of tab for values from (-10) to (0);
create table tab_2 partition of tab for values from (0) to (10);
create publication pub for table tab_1, tab_2 with (publish_via_partition_root = true);
```

For now, changes are replicated as leaf table's one in above case. And I think
it is the expected behavior because users expressly specifies them.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Wed, Oct 23, 2024 at 3:42 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Problem also happens if publication is created via FOR ALL TABLES /
FOR TABLES IN SCHEMA (without directly including partition head into
publication).

Thanks for pointing out. I also confirmed this issue can happen for
FOR ALL TABLES/FOR TABLES IN SCHEMA publications. I modified a documentation.

I don't think we need to mention this in docs, the following change:
"For now, changes are replicated as leaf table's one in above case.
And I think it is the expected behavior because users expressly
specifies them." added by you appears odd to me.

--
With Regards,
Amit Kapila.

Hi Kuroda-san,

This is only a caution for ALTER of 'publish_via_partition_root', so
that should be immediately clear up-front in the first few words of
the text.

Also, the current explanation appears to give more details about how
and why it happens than I felt was necessary. e.g. this is for user
documentation, not for a code comment. Basically, I'm wondering if the
whole thing could be "dumbed down" a little bit, keeping only the
information essential so the users can protect themselves. Maybe
something a bit more like below?

SUGGESTION
<caution>
<para>
Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.
</para>
<para>
This problem can be avoided by refraining from modifying
partition leaf tables
after the <command>ALTER PUBLICATION ... SET</command> until the
<link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... REFRESH PUBLICATION</command></link>
is executed, and by only refreshing using the <literal>copy_data
= off</literal> option.
</para>
</caution>

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Fri, Oct 25, 2024 at 6:12 AM Peter Smith <smithpb2250@gmail.com> wrote:

Hi Kuroda-san,

This is only a caution for ALTER of 'publish_via_partition_root', so
that should be immediately clear up-front in the first few words of
the text.

Also, the current explanation appears to give more details about how
and why it happens than I felt was necessary. e.g. this is for user
documentation, not for a code comment. Basically, I'm wondering if the
whole thing could be "dumbed down" a little bit, keeping only the
information essential so the users can protect themselves. Maybe
something a bit more like below?

SUGGESTION
<caution>
<para>
Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.
</para>

This appears precise but lacks the key information that the problem
can happen when a partitioned root table is specified as a replication
target. So, how about one of the following:

* Altering the <literal>publish_via_partition_root</literal> parameter
when the partition root table is specified as the replication target
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.

* Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables. Note this
happens only when the partition root table is specified as the
replication target.

<para>
This problem can be avoided by refraining from modifying
partition leaf tables
after the <command>ALTER PUBLICATION ... SET</command> until the
<link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... REFRESH PUBLICATION</command></link>
is executed, and by only refreshing using the <literal>copy_data
= off</literal> option.
</para>
</caution>

We can keep this part as you proposed.

--
With Regards,
Amit Kapila.

On Fri, Oct 25, 2024 at 2:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Oct 25, 2024 at 6:12 AM Peter Smith <smithpb2250@gmail.com> wrote:

Hi Kuroda-san,

This is only a caution for ALTER of 'publish_via_partition_root', so
that should be immediately clear up-front in the first few words of
the text.

Also, the current explanation appears to give more details about how
and why it happens than I felt was necessary. e.g. this is for user
documentation, not for a code comment. Basically, I'm wondering if the
whole thing could be "dumbed down" a little bit, keeping only the
information essential so the users can protect themselves. Maybe
something a bit more like below?

SUGGESTION
<caution>
<para>
Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.
</para>

This appears precise but lacks the key information that the problem
can happen when a partitioned root table is specified as a replication
target. So, how about one of the following:

* Altering the <literal>publish_via_partition_root</literal> parameter
when the partition root table is specified as the replication target
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.

* Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables. Note this
happens only when the partition root table is specified as the
replication target.

+1 for the second one. (but maybe say "a partition root table" instead
of "the partition root table")

Dear Amit, Peter,

SUGGESTION
<caution>
<para>
Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.
</para>

This appears precise but lacks the key information that the problem
can happen when a partitioned root table is specified as a replication
target. So, how about one of the following:

* Altering the <literal>publish_via_partition_root</literal> parameter
when the partition root table is specified as the replication target
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables.

* Altering the <literal>publish_via_partition_root</literal> parameter
can lead to data loss or duplication at the subscriber because it
changes the identity and schema of the published tables. Note this
happens only when the partition root table is specified as the
replication target.

<para>
This problem can be avoided by refraining from modifying
partition leaf tables
after the <command>ALTER PUBLICATION ... SET</command> until

the

<link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... REFRESH PUBLICATION</command></link>
is executed, and by only refreshing using the <literal>copy_data
= off</literal> option.
</para>
</caution>

We can keep this part as you proposed.

Thanks for suggestions. I updated as you pointed out. I removed a comma from
"is executed, and..." because my Grammarly said like that.
PSA new version.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Fri, Oct 25, 2024 at 12:21 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Thanks for suggestions. I updated as you pointed out. I removed a comma from
"is executed, and..." because my Grammarly said like that.
PSA new version.

Pushed.

--
With Regards,
Amit Kapila.