Horton

Re: Skipping logical replication transactions on subscriber side

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Euler Taveira <euler@eulerto.com>, "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, "David G. Johnston" <david.g.johnston@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, vignesh C <vignesh21@gmail.com>, Greg Nancarrow <gregn4422@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Alexey Lesovsky <lesovsky@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-04-04T06:26:20Z
Lists: pgsql-hackers 
On Mon, Apr 04, 2022 at 08:20:08AM +0530, Amit Kapila wrote:
> On Mon, Apr 4, 2022 at 8:01 AM Noah Misch <noah@leadboat.com> wrote:
> > On Mon, Apr 04, 2022 at 10:28:30AM +0900, Masahiko Sawada wrote:
> > > On Sun, Apr 3, 2022 at 9:45 AM Noah Misch <noah@leadboat.com> wrote:
> > > > On Sat, Apr 02, 2022 at 08:44:45PM +0900, Masahiko Sawada wrote:
> > > > > On Sat, Apr 2, 2022 at 7:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > > > On Sat, Apr 2, 2022 at 1:43 PM Noah Misch <noah@leadboat.com> wrote:
> > > > > > > Some options:
> > > > > > > - Move subskiplsn after subdbid, so it's always aligned anyway.  I've
> > > > > > >   confirmed that this lets the test pass, in 44s.
> >
> > > --- a/src/include/catalog/pg_subscription.h
> > > +++ b/src/include/catalog/pg_subscription.h
> > > @@ -54,6 +54,17 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW
> > >
> > >       Oid                     subdbid BKI_LOOKUP(pg_database);        /* Database the
> > >                                                                                                        * subscription is in. */
> > > +
> > > +     /*
> > > +      * All changes finished at this LSN are skipped.
> > > +      *
> > > +      * Note that XLogRecPtr, pg_lsn in the catalog, is 8-byte alignment
> > > +      * (TYPALIGN_DOUBLE) and it does not match the alignment on some platforms
> > > +      * such as AIX.  Therefore subskiplsn needs to be placed here so it is
> > > +      * always aligned.
> >
> > I'm reading this comment as saying that TYPALIGN_DOUBLE is always 8 bytes, but
> > the problem arises precisely because TYPALIGN_DOUBLE==4 on AIX.
> 
> 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.

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.  If adding such a test would materially delay getting the
buildfarm green, putting the comment in pg_subscription.h works for me.

Commits

  1. Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.

  2. Reorder subskiplsn in pg_subscription to avoid alignment issues.

  3. Add ALTER SUBSCRIPTION ... SKIP.

  4. Optionally disable subscriptions on error.

  5. Update docs of logical replication for commit 8d74fc96db.

  6. Respect permissions within logical replication.

  7. Fix regression test failure caused by commit 8d74fc96db.

  8. Add a view to show the stats of subscription workers.

  9. Add logical change details to logical replication worker errcontext.

  10. Rename LOGICAL_REP_MSG_STREAM_END to LOGICAL_REP_MSG_STREAM_STOP.

  11. Fix typo in protocol.sgml.

  12. Remove unused argument in apply_handle_commit_internal().

  13. Fix replication of in-progress transactions in tablesync worker.

  14. Reorder pg_sequence columns to avoid alignment issue