On Thurs, Jul 28, 2022 at 21:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
Thanks for your comments and opinions.
> On Wed, Jul 27, 2022 at 1:27 PM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.com> wrote:
> > BTW, I think the foreign key won't take effect in subscriber's apply worker by
> > default. Because we set session_replication_role to 'replica' in apply worker
> > which prevent the FK trigger function to be executed(only the trigger with
> > FIRES_ON_REPLICA flag will be executed in this mode). User can only alter the
> > trigger to enable it on replica mode to make the foreign key work. So, ISTM,
> we
> > won't hit this ERROR frequently.
> >
> > And based on this, another comment about the patch is that it seems
> unnecessary
> > to directly check the FK returned by RelationGetFKeyList. Checking the actual
> FK
> > trigger function seems enough.
> >
>
> That is correct. I think it would have been better if we can detect
> that publisher doesn't have FK but the subscriber has FK as it can
> occur only in that scenario. If that requires us to send more
> information from the publisher, we can leave it for now (as this
> doesn't seem to be a frequent scenario) and keep a simpler check based
> on subscriber schema.
>
> I think we should add a test as mentioned by you above so that if
> tomorrow one tries to remove the FK check, we have a way to know.
> Also, please add comments and tests for additional checks related to
> constraints in the patch.
>
> [1] - https://www.postgresql.org/message-
> id/CAA4eK1JwahU_WuP3S%2B7POqta%3DPhm_3gxZeVmJuuoUq1NV%3DkrXA
> %40mail.gmail.com
I added some test cases that cause indefinite waits without additional checks
related to constraints. (please see file 032_streaming_apply.pl in 0003-patch)
I also added some comments for FK check and why we need these checks.
In addition, I found another two scenarios that could cause infinite waits, so
I made the following changes:
1. I check the default values for the columns that only in subscriber-side.
(Previous versions only checked for columns that existed in both
publisher-side and subscriber-side.)
2. When using an apply background worker, the check needs to be performed not
only in the apply background worker, but also in the main apply worker.
I also did some other improvements based on the suggestions posted in this
thread. Attach the new patches.
Regards,
Wang wei
