Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Fri, May 29, 2020 at 8:31 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> Apart from this one more fix in 0005, basically, CheckLiveXid was
> never reset, so I have fixed that as well.
>

I have made a number of modifications in the 0001 patch and attached
is the result. I have changed/added comments, done some cosmetic
cleanup, and ran pgindent. The most notable change is to remove the
below code change:
DecodeXactOp()
{
..
- * However, it's critical to process XLOG_XACT_ASSIGNMENT records even
+ * However, it's critical to process records with subxid assignment even
* when the snapshot is being built: it is possible to get later records
* that require subxids to be properly assigned.
*/
if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT &&
- info != XLOG_XACT_ASSIGNMENT)
+ !TransactionIdIsValid(XLogRecGetTopXid(r)))
..
}

I have not only removed the change done by the patch but the check
related to XLOG_XACT_ASSIGNMENT as well. That check has been added by
commit bac2fae05c to ensure that we process XLOG_XACT_ASSIGNMENT even
if snapshot state is not SNAPBUILD_FULL_SNAPSHOT. Now, with this
patch that is not required because we are making the subtransaction
and top-level transaction much earlier than this. I have verified
that it doesn't reopen the bug by running the test provided in the
original report [1].

Let me know what you think of the changes? If you find them okay,
then feel to include them in the next patch-set.

[1] - https://www.postgresql.org/message-id/CAONYFtOv%2BEr1p3WAuwUsy1zsCFrSYvpHLhapC_fMD-zNaRWxYg%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com