From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com> |
Cc: | Ajin Cherian <itsajin(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Truncate in synchronous logical replication failed |
Date: | 2021-04-22 09:33:22 |
Message-ID: | CAA4eK1Jg2fB2phyWGxHV83qkE5pNuf82kU21Zzif6dpXDC+zbQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Apr 20, 2021 at 9:00 AM osumi(dot)takamichi(at)fujitsu(dot)com
<osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
>
> On Tuesday, April 20, 2021 10:53 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> >
> > I reviewed the patch, ran make check, no issues. One minor comment:
> >
> > Could you add the comment similar to RelationGetIndexAttrBitmap() on why
> > the redo, it's not very obvious to someone reading the code, why we are
> > refetching the index list here.
> >
> > + /* Check if we need to redo */
> >
> > + newindexoidlist = RelationGetIndexList(relation);
> Yeah, makes sense. Fixed.
I don't think here we need to restart to get a stable list of indexes
as we do in RelationGetIndexAttrBitmap. The reason is here we build
the cache entry using a historic snapshot and all the later changes
are absorbed while decoding WAL. I have updated that and modified few
comments in the attached patch. Can you please test this in
clobber_cache_always mode? I think just testing
subscription/t/010_truncate.pl would be sufficient.
Now, this bug exists in prior versions (>= v11) where we have
introduced decoding of Truncate. Do we want to backpatch this? As no
user has reported this till now apart from Tang who I think got it
while doing some other patch testing, we might want to just push it in
HEAD. I am fine either way. Petr, others, do you have any opinion on
this matter?
--
With Regards,
Amit Kapila.
Attachment | Content-Type | Size |
---|---|---|
truncate_in_synchronous_logical_replication_v05.patch | application/octet-stream | 5.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2021-04-22 09:56:25 | Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY |
Previous Message | Julien Rouhaud | 2021-04-22 09:23:18 | Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view? |