On Wed, Jan 12, 2022 at 6:04 PM osumi.takami...@fujitsu.com <osumi.takami...@fujitsu.com> wrote: > > On Thursday, December 23, 2021 6:37 PM Wang, Wei/王 威 <wangw.f...@fujitsu.com> > wrote: > > On Wednesday, December 22, 2021 10:30 PM osumi.takami...@fujitsu.com > > <osumi.takami...@fujitsu.com> wrote: > > > On Wednesday, December 22, 2021 8:38 PM I wrote: > > > > Do we expect these commit counts which come from empty transactions ? > > > This is another issue discussed in [1] where the patch in the thread > > > is a work in progress, I think. > > > ...... > > > IMHO, the conclusion is we are currently in the middle of fixing the > > > behavior. > > > > Thank you for telling me this. > > After applying v19-* and > > v15-0001-Skip-empty-transactions-for-logical-replication.patch, > > I retested v19-* patches. The result of previous case looks good to me. > > > > But the results of following cases are also similar to previous unexpected > > result > > which increases commit_count or abort_count unexpectedly. > > [1] > > (Based on environment in the previous example, set TWO_PHASE=true) > > [Publisher] begin; insert into replica_test1 values(1,'1'); prepare > > transaction > > 'id'; commit prepared 'id'; > > > > In subscriber side, the commit_count of two records(sub1 and sub2) is > > increased. > > > > [2] > > (Based on environment in the previous example, set STREAMING=on) > > [Publisher] begin; INSERT INTO replica_test1 SELECT i, md5(i::text) FROM > > generate_series(1, 5000) s(i); commit; > > > > In subscriber side, the commit_count of two records(sub1 and sub2) is > > increased. > > > > [3] > > (Based on environment in the previous example, set TWO_PHASE=true) > > [Publisher] begin; insert into replica_test1 values(1,'1'); prepare > > transaction > > 'id'; rollback prepared 'id'; > > > > In subscriber side, the abort_count of two records(sub1 and sub2) is > > increased. > > > > I think the problem maybe is the patch you mentioned > > (Skip-empty-transactions-for-logical-replication.patch) is not finished yet. > > Share this information here. > Hi, thank you for your report. > > Yes. As the patch's commit message mentions, the patch doesn't > cover streaming and two phase transactions. > > In the above reply, I said that this was an independent issue > and we were in the middle of the modification of the behavior, > but empty transaction turned out to be harmful enough for this feature. >
Isn't that because of this patch? I mean the patch is reporting count even when during apply we haven't started any transaction. In particular, if you would have reported stats from apply_handle_commit_internal() when IsTransactionState() returns true, then it shouldn't have updated the stats for an empty transaction. Similarly, I see for rollback_prepared, you don't need to report stats if there is no prepared transaction to rollback. I think for commit_prepare case, we can't do much for empty xacts but why would that be a problem of this patch? I think as far as this patch goes, it reports the number of completed xacts (committed/aborted) in the subscription worker, so, it doesn't need to handle any special cases like empty transactions. -- With Regards, Amit Kapila.
