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/