On Mon, Nov 23, 2020 at 3:13 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Nov 23, 2020 at 10:51 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Sat, Nov 21, 2020 at 12:23 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > 2.
> > > @@ -902,7 +906,9 @@ apply_handle_stream_abort(StringInfo s)
> > > {
> > > /* Cleanup the subxact info */
> > > cleanup_subxact_info();
> > > - CommitTransactionCommand();
> > > +
> > > + if (!am_tablesync_worker())
> > > + CommitTransactionCommand();
> > >
> > > Here, also you can add a comment: "/* The synchronization worker runs
> > > in single transaction. */"
> > >
> >
> > Done
> >
>
> Okay, thanks. I have slightly changed the comments and moved the newly
> added function in the attached patch.
Okay, looks good to me.
I have tested the reported
> scenario and additionally verified that the fix is good even if the
> tablesync worker processed the partial transaction due to streaming.
> This won't do any harm because later apply worker will replay the
> entire transaction. This could be a problem if the apply worker also
> tries to stream the transaction between the SUBREL_STATE_CATCHUP and
> SUBREL_STATE_SYNCDONE state because then apply worker might have
> skipped applying the partial transactions processed by tablesync
> worker. But, I have checked that the apply worker waits for sync
> worker to complete its processing between these two states.
Right
See
> process_syncing_tables_for_apply. Does this make sense?
Yes, it makes sense to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com