On Thu, Oct 21, 2021 at 9:11 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Oct 20, 2021 at 8:49 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > >
> > > Does MarkTopTransactionIdLogged() have to be inside XLogInsertRecord's
> > > critical section?
> >
> > I think this function is doing somewhat similar things to what we are
> > doing in MarkCurrentTransactionIdLoggedIfAny() so put at the same
> > place. But I don't see any reason for this to be in the critical
> > section.
> >
>
> Yeah, I also don't see any reason for this to be in the critical
> section but it might be better to keep both together. So, if we want
> to keep MarkTopTransactionIdLogged() out of the critical section in
> this patch then we should move the existing function
> MarkCurrentTransactionIdLoggedIfAny() in a separate patch so that
> future readers doesn't get confused as to why one of these is in the
> critical section and other is not. OTOH, we can move
> MarkCurrentTransactionIdLoggedIfAny() out of the critical section in
> this patch itself but that appears like an unrelated change and we may
> or may not want to back-patch the same.
>
v5-0001, incorporates all the comment fixes suggested by Alvaro. and
0001 is an additional patch which moves
MarkCurrentTransactionIdLoggedIfAny(), out of the critical section.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com