public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
Date: Thu, 6 Jul 2023 14:06:32 +0530
Message-ID: <CAA4eK1LgUqK8daxX=FZ8F_O4DBJC5YxWixzoTX_PH2JFKm0tQQ@mail.gmail.com> (raw)
In-Reply-To: <CAExHW5tcfvvwrKP4TtHFetBd0mc3CPpjsSDbjz3HYQJCcGc4JA@mail.gmail.com>
References: <CAExHW5ujRhbOz6_aTq_jQA8NjeFqq9d_8G9viShWvXx8gdSXiQ@mail.gmail.com>
<CAA4eK1JLRiW_Mkps+oGu_a1DZHJqhoXV9Ru33B4hV5UYUtLPJg@mail.gmail.com>
<CALDaNm1bfwt=hmZD7V17t-giJLw3kndLBxLzSD+oSXNfMmGVsw@mail.gmail.com>
<CAExHW5sqY8AS5WqyTD1v=V+pAZvtHXcZ5++2z1fn7NcpBmhj_g@mail.gmail.com>
<CALDaNm0fWoS7eX9o3iVq8NwynpvAKKFK0YPEXWDTgBEOOjvFkw@mail.gmail.com>
<OS0PR01MB57161E8D61ACCF34D505FBDA9425A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CALDaNm14bBNi60JaveK-WDgjYsDxa5ArRWzj2JrjQgc9CBzYiw@mail.gmail.com>
<CAA4eK1J-_g8ruuOKTXA7sGjRASBG7XsXpHzpz324ygEd+HmsvA@mail.gmail.com>
<CALDaNm3Hj72XtEcn_nRm4t4c59gaYCJk=-aeaJT47ktoAmBUNQ@mail.gmail.com>
<CAA4eK1LA=gb0xyg+-93WJOX1EEJE8qroPQnNw==DreuW2VDm7g@mail.gmail.com>
<CAA4eK1KPvbtpCJUuU6ZaZ=6BuBEvAzLCHNniNG98g0tOo3CkNA@mail.gmail.com>
<CAExHW5tcfvvwrKP4TtHFetBd0mc3CPpjsSDbjz3HYQJCcGc4JA@mail.gmail.com>
On Wed, Jul 5, 2023 at 7:20 PM Ashutosh Bapat
<[email protected]> wrote:
>
> On Wed, Jul 5, 2023 at 2:29 PM Amit Kapila <[email protected]> wrote:
> >
> > On Wed, Jul 5, 2023 at 2:28 PM Amit Kapila <[email protected]> wrote:
> > >
> > > On Mon, Jul 3, 2023 at 4:49 PM vignesh C <[email protected]> wrote:
> > > >
> > > > +1 for the first version patch, I also felt the first version is
> > > > easily understandable.
> > > >
> > >
> > > Okay, please find the slightly updated version (changed a comment and
> > > commit message). Unless there are more comments, I'll push this in a
> > > day or two.
> > >
> >
> > oops, forgot to attach the patch.
>
> I still think that we need to do something so that a new call to
> pg_output_begin() automatically takes care of the conditions under
> which it should be called. Otherwise, we will introduce a similar
> problem in some other place in future.
>
AFAIU, this problem is because we forget to conditionally call
pg_output_begin() from pg_decode_message() which can happen with or
without moving conditions inside pg_output_begin(). Also, it shouldn't
be done at the expense of complexity. I find the check added by
Vignesh's v2 patch (+ if (!(last_write ^ data->skip_empty_xacts) ||
txndata->xact_wrote_changes)) a bit difficult to understand and more
error-prone. The others like Hou-San also couldn't understand unless
Vignesh gave an explanation. I also thought of avoiding that check.
Basically, IIUC, the check is added because the patch also removed
'data->skip_empty_xacts' check from
pg_decode_begin_txn()/pg_decode_begin_prepare_txn(). Now, if retain
those checks then it is probably okay but again the similar checks are
still split and that doesn't appear to be better than the v1 or v3
patch version. I am not against improving code in this area and
probably we can consider doing it as a separate patch if we have
better ideas instead of combining it with this patch.
--
With Regards,
Amit Kapila.
view thread (7+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
In-Reply-To: <CAA4eK1LgUqK8daxX=FZ8F_O4DBJC5YxWixzoTX_PH2JFKm0tQQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox