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

On Tue, Jun 28, 2022 11:22 AM Wang, Wei/王 威 <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
>
> I also improved patches as suggested by Peter-san in [1] and [2].
> Thanks for Shi Yu to improve the patches by addressing the comments in [2].
>
> Attach the new patches.
>

Thanks for updating the patch.

Here are some comments.

0001 patch
==============
1.
+ /* Check If there are free worker slot(s) */
+ LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);

I think "Check If" should be "Check if".

0003 patch
==============
1.
Should we call apply_bgworker_relation_check() in apply_handle_truncate()?

0004 patch
==============
1.
@@ -3932,6 +3958,9 @@ start_apply(XLogRecPtr origin_startpos)
}
PG_CATCH();
{
+ /* Set the flag that we will retry later. */
+ set_subscription_retry(true);
+
if (MySubscription->disableonerr)
DisableSubscriptionAndExit();
Else

I think we need to emit the error and recover from the error state before
setting the retry flag, like what we do in DisableSubscriptionAndExit().
Otherwise if an error is detected when setting the retry flag, we won't get the
error message reported by the apply worker.

Regards,
Shi yu