RE: Failed transaction statistics to measure the logical replication progress

From: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
To: 'Dilip Kumar' <dilipbalaut(at)gmail(dot)com>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Failed transaction statistics to measure the logical replication progress
Date: 2021-11-15 09:32:47
Message-ID: TYCPR01MB837383AD25724677A0A99AE6ED989@TYCPR01MB8373.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wednesday, November 10, 2021 6:13 PM I wrote:
> On Wednesday, November 10, 2021 3:43 PM Dilip Kumar
> <dilipbalaut(at)gmail(dot)com> wrote:
> > On Tue, Nov 9, 2021 at 5:05 PM osumi(dot)takamichi(at)fujitsu(dot)com
> > <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> > > On Tuesday, November 9, 2021 12:08 PM Greg Nancarrow
> > <gregn4422(at)gmail(dot)com> wrote:
> > > > On Fri, Nov 5, 2021 at 7:11 PM osumi(dot)takamichi(at)fujitsu(dot)com
> > > > <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> > > > >
> > > >
> > > > I did a quick scan through the latest v8 patch and noticed the
> > > > following
> > things:
> > > I appreciate your review !
> > I have reviewed some part of the patch and I have a few comments
> I really appreciate your attention and review.
...
> > 3.
> > + {
> > + size += *extra_data->stream_write_len;
> > + add_apply_error_context_xact_size(size);
> > + return;
> > + }
> >
> > From apply_handle_insert(), we are calling update_apply_change_size(),
> > and inside this function we are dereferencing
> *extra_data->stream_write_len.
> > Basically, stream_write_len is in integer pointer and the caller
> > hasn't allocated memory for that and inside update_apply_change_size,
> > we are directly dereferencing the pointer, how this can be correct.
...
> I'll just delete the top part that handles streaming bytes calculation in the
> update_apply_change_size().
> It's because now that there is a specific structure to recognize each streaming
> xid and save transaction size there, which makes the top part in question
> useless.
>
> > And I also see that in the
> > whole patch stream_write_len, is never used as lvalue so without
> > storing anything into this why are we trying to use this as rvalue
> > here? This is clearly an issue.
> As described above, I'll fix this part and related codes mainly streaming related
> codes in the next version.
Removed this deadcodes.

Please have a look at [1]

[1] - https://www.postgresql.org/message-id/TYCPR01MB8373FEB287F733C81C1E4D42ED989%40TYCPR01MB8373.jpnprd01.prod.outlook.com

Best Regards,
Takamichi Osumi

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-11-15 09:36:54 Re: row filtering for logical replication
Previous Message osumi.takamichi@fujitsu.com 2021-11-15 09:31:20 RE: Failed transaction statistics to measure the logical replication progress