Re: Optionally automatically disable logical replication subscriptions on error - Mailing list pgsql-hackers

On Tue, Mar 8, 2022 at 5:07 PM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> Kindly have a look at v30.

Thank you for updating the patch. Here are some comments:

+   /*
+    * Allocate the origin name in long-lived context for error context
+    * message.
+    */
+   ReplicationOriginNameForTablesync(MySubscription->oid,
+                                     MyLogicalRepWorker->relid,
+                                     originname,
+                                     sizeof(originname));
+   apply_error_callback_arg.origin_name = MemoryContextStrdup(ApplyContext,
+                                                              originname);

I think it's better to set apply_error_callback_arg.origin_name in the
caller rather than in start_table_sync(). Apply workers set
apply_error_callback_arg.origin_name there and it's not necessarily
necessary to do that in this function.

Even if we want to do that, I think it's not necessary to pass
originname to start_table_sync(). It's a local variable and used only
to temporarily store the tablesync worker's origin name.

---
It might have already been discussed but the worker disables the
subscription on an error but doesn't work for a fatal. Is that
expected or should we handle that too?

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/