On Mon, Apr 4, 2022 at 8:01 AM Noah Misch <n...@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 <n...@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.kapil...@gmail.com> > > > > wrote: > > > > > On Sat, Apr 2, 2022 at 1:43 PM Noah Misch <n...@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? > > + */ > > + XLogRecPtr subskiplsn; > > + > > NameData subname; /* Name of the subscription */ > > > > Oid subowner BKI_LOOKUP(pg_authid); /* Owner of > > the subscription */ > > @@ -71,9 +82,6 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) > > BKI_SHARED_RELATION BKI_ROW > > bool subdisableonerr; /* True if a worker error > > should cause the > > * > > subscription to be disabled */ > > > > - XLogRecPtr subskiplsn; /* All changes finished at > > this LSN are > > - * skipped */ > > Some code sites list pg_subscription fields in field order. Please update > them so they continue to list fields in field order. CreateSubscription() is > one example. > Another minor point is that I think it is better to use DatumGetLSN to read this in GetSubscription as we use LSNGetDatum while storing it. I am not sure if there is any direct problem due to this but that looks consistent to me. -- With Regards, Amit Kapila.
