Re: Skipping logical replication transactions on subscriber side

From: Noah Misch <noah(at)leadboat(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Alexey Lesovsky <lesovsky(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Skipping logical replication transactions on subscriber side
Date: 2022-04-05 01:46:20
Message-ID: 20220405014620.GB3827588@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 05, 2022 at 10:13:06AM +0900, Masahiko Sawada wrote:
> On Tue, Apr 5, 2022 at 9:21 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Mon, Apr 04, 2022 at 06:55:45PM +0900, Masahiko Sawada wrote:
> > > On Mon, Apr 4, 2022 at 3:26 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > > On Mon, Apr 04, 2022 at 08:20:08AM +0530, Amit Kapila wrote:
> > > > > How about a comment like: "It has to be kept at 8-byte alignment
> > > > > boundary so as to be accessed directly via C struct as it uses
> > > > > TYPALIGN_DOUBLE for storage which has 4-byte alignment on platforms
> > > > > like AIX."? Can you please suggest a better comment if you don't like
> > > > > this one?
> > > >
> > > > I'd write it like this, though I'm not sure it's an improvement on your words:
> > > >
> > > > When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
> > > > some of the C types that correspond to TYPALIGN_DOUBLE SQL types. To ensure
> > > > catalog C struct layout matches catalog tuple layout, arrange for the tuple
> > > > offset of each fixed-width, attalign='d' catalog column to be divisible by 8
> > > > unconditionally. Keep such columns before the first NameData column of the
> > > > catalog, since packagers can override NAMEDATALEN to an odd number.
> > >
> > > Thanks!
> > >
> > > >
> > > > The best place for such a comment would be in one of
> > > > src/test/regress/sql/*sanity*.sql, next to a test written to detect new
> > > > violations.
> > >
> > > Agreed.
> > >
> > > IIUC in the new test, we would need a new SQL function to calculate
> > > the offset of catalog columns including padding, is that right? Or do
> > > you have an idea to do that by using existing functionality?
> >
> > Something like this:
> >
> > select
> > attrelid::regclass,
> > attname,
> > array(select typname
> > from pg_type t join pg_attribute pa on t.oid = pa.atttypid
> > where pa.attrelid = a.attrelid and pa.attnum > 0 and pa.attnum < a.attnum order by pa.attnum) AS types_before,
> > (select sum(attlen)
> > from pg_type t join pg_attribute pa on t.oid = pa.atttypid
> > where pa.attrelid = a.attrelid and pa.attnum > 0 and pa.attnum < a.attnum) AS len_before
> > from pg_attribute a
> > join pg_class c on c.oid = attrelid
> > where attalign = 'd' and relkind = 'r' and attnotnull and attlen <> -1
> > order by attrelid::regclass::text, attnum;
> > attrelid │ attname │ types_before │ len_before
> > ─────────────────┼──────────────┼─────────────────────────────────────────────┼────────────
> > pg_sequence │ seqstart │ {oid,oid} │ 8
> > pg_sequence │ seqincrement │ {oid,oid,int8} │ 16
> > pg_sequence │ seqmax │ {oid,oid,int8,int8} │ 24
> > pg_sequence │ seqmin │ {oid,oid,int8,int8,int8} │ 32
> > pg_sequence │ seqcache │ {oid,oid,int8,int8,int8,int8} │ 40
> > pg_subscription │ subskiplsn │ {oid,oid,name,oid,bool,bool,bool,char,bool} │ 81
> > (6 rows)
> >
> > That doesn't count padding, but hazardous column changes will cause a diff in
> > the output.
>
> Yes, in this case, we can detect the violated column order even
> without considering padding. On the other hand, I think this
> calculation could not detect some patterns of order. For instance,
> suppose the column order is {oid, bool, bool, oid, bool, bool, oid,
> int8}, the len_before is 16 but offset of int8 column including
> padding is 20 on ALIGNOF_DOUBLE==4 environment.

Correct. Feel free to make it more precise. If you do want to add a
function, it could be a regress.c function rather than an always-installed
part of PostgreSQL. Again, getting the buildfarm green is a priority; we can
always add tests later.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2022-04-05 01:54:37 Re: Handle infinite recursion in logical replication setup
Previous Message Masahiko Sawada 2022-04-05 01:40:04 Re: Showing I/O timings spent reading/writing temp buffers in EXPLAIN