On Wed, Feb 1, 2023 20:07 PM Melih Mutlu <m.melihmu...@gmail.com> wrote:
> Thanks for pointing out this review. I somehow skipped that, sorry.
>
> Please see attached patches.
Thanks for updating the patch set.
Here are some comments.
1. In the function ApplyWorkerMain.
+ /* This is main apply worker */
+ run_apply_worker(&options, myslotname, originname,
sizeof(originname), &origin_startpos);
I think we need to keep the worker name as "leader apply worker" in the comment
like the current HEAD.
---
2. In the function LogicalRepApplyLoop.
+ * can be reused, we need to take care of
memory contexts here
+ * before moving to sync a table.
+ */
+ if (MyLogicalRepWorker->ready_to_reuse)
+ {
+
MemoryContextResetAndDeleteChildren(ApplyMessageContext);
+ MemoryContextSwitchTo(TopMemoryContext);
+ return;
+ }
I think in this case we also need to pop the error context stack before
returning. Otherwise, I think we might use the wrong callback
(apply error_callback) after we return from this function.
---
3. About the function UpdateSubscriptionRelReplicationSlot.
This newly introduced function UpdateSubscriptionRelReplicationSlot does not
seem to be invoked. Do we need this function?
Regards,
Wang Wei
