Horton
Menu

RE: Perform streaming logical transactions by background workers and parallel apply

Takamichi Osumi (Fujitsu) <osumi.takamichi@fujitsu.com>

View thread
From: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>
To: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-05-29T12:25:12Z
Lists: pgsql-hackers 
On Wednesday, May 25, 2022 11:25 AM wangw.fnst@fujitsu.com <wangw.fnst@fujitsu.com> wrote:
> Attach the patches. (Did not change v6-0001 and v6-0002.)
Hi,


Some review comments on the new patches from v6-0001 to v6-0004.

<v6-0001>

(1) create_subscription.sgml

+          the transaction is committed. Note that if an error happens when
+          applying changes in a background worker, it might not report the
+          finish LSN of the remote transaction in the server log.

I suggest to add a couple of sentences like below
to the section of logical-replication-conflicts in logical-replication.sgml.

"
Setting streaming mode to 'apply' can export invalid LSN as
finish LSN of failed transaction. Changing the streaming mode and
making the same conflict writes the finish LSN of the
failed transaction in the server log if required.
"

(2) ApplyBgworkerMain


+       PG_TRY();
+       {
+               LogicalApplyBgwLoop(mqh, pst);
+       }
+       PG_CATCH();
+       {

...

+               pgstat_report_subscription_error(MySubscription->oid, false);
+
+               PG_RE_THROW();
+       }
+       PG_END_TRY();


When I stream a transaction in-progress and it causes an error(duplication error),
seemingly the subscription stats (values in pg_stat_subscription_stats) don't
get updated properly. The 2nd argument should be true for apply error.

Also, I observe that both apply_error_count and sync_error_count
get updated together by error. I think we need to check this point as well.


<v6-0003>


(3) logicalrep_write_attrs

+       if (rel->rd_rel->relhasindex)
+       {
+               List       *indexoidlist = RelationGetIndexList(rel);
+               ListCell   *indexoidscan;
+               foreach(indexoidscan, indexoidlist)

and

+                       if (indexRel->rd_index->indisunique)
+                       {
+                               int             i;
+                               /* Add referenced attributes to idindexattrs */
+                               for (i = 0; i < indexRel->rd_index->indnatts; i++)

We don't have each blank line after variable declarations.
There might be some other codes where this point can be applied.
Please check.


(4)

+       /*
+        * If any unique index exist, check that they are same as remoterel.
+        */
+       if (!rel->sameunique)
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot replicate relation with different unique index"),
+                                errhint("Please change the streaming option to 'on' instead of 'apply'.")));


When I create a logical replication setup with different constraints
and let streaming of in-progress transaction run,
I keep getting this error.

This should be documented as a restriction or something,
to let users know the replication progress can't go forward by
any differences written like in the commit-message in v6-0003.

Also, it would be preferable to test this as well, if we
don't dislike having TAP tests for this.


Best Regards,
	Takamichi Osumi

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix invalid memory access during the shutdown of the parallel apply worker.

  2. Fix assertion failure in apply worker.

  3. Use elog to report unexpected action in handle_streamed_transaction().

  4. Use appropriate wait event when sending data in the apply worker.

  5. Allow the logical_replication_mode to be used on the subscriber.

  6. Rename GUC logical_decoding_mode to logical_replication_mode.

  7. Display the leader apply worker's PID for parallel apply workers.

  8. Improve the code to decide and process the apply action.

  9. Document the newly added wait events added by commit 216a784829.

  10. Perform apply of large transactions by parallel workers.

  11. Wake up a subscription's replication worker processes after DDL.

  12. Add copyright notices to meson files

  13. Better document logical replication parameters

  14. Add a common function to generate the origin name.

  15. Harmonize parameter names in storage and AM code.

  16. Avoid using list_length() to test for empty list.

  17. Improve two comments related to a boolean DefElem's value

  18. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  19. Fix data inconsistency between publisher and subscriber.

  20. Fix cache look-up failures while applying changes in logical replication.