Horton

Re: Minimal logical decoding on standbys

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

View thread
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Amit Khandekar <amitdkhan.pg@gmail.com>, fabriziomello@gmail.com, tushar <tushar.ahuja@enterprisedb.com>, Rahila Syed <rahila.syed@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-12-22T07:50:34Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Reduce the log level in 035_standby_logical_decoding.pl.

  2. 035_standby_logical_decoding: Add missing waits for replication

  3. For cascading replication, wake physical and logical walsenders separately

  4. Handle logical slot conflicts on standby

  5. Support invalidating replication slots due to horizon and wal_level

  6. Prevent use of invalidated logical slot in CreateDecodingContext()

  7. Replace replication slot's invalidated_at LSN with an enum

  8. Pass down table relation into more index relation functions

  9. Assert only valid flag bits are passed to visibilitymap_set()

  10. Remove unused _bt_delitems_delete() argument.

  11. Add xl_btree_delete optimization.

Attachments

Hi,

On 12/21/22 10:06 AM, Drouvot, Bertrand wrote:
> Hi,
> 
> On 12/20/22 10:41 PM, Robert Haas wrote:
>> On Tue, Dec 20, 2022 at 3:39 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> I guess whatever else we
>> do here, we should fix the comments.
>>
> 
> Agree, please find attached a patch proposal doing so.
> 
> 
>> Bottom line is that I think the two cases that have alignment issues
>> as coded are xl_hash_vacuum_one_page and gistxlogDelete. Everything
>> else is OK, as far as I can tell right now.
>>
> 
> Thanks a lot for the repro(s) and explanations! That's very useful/helpful.
> 
> Based on your discovery about the wrong comments above, I'm now tempted to fix those 2 alignment issues
> by using a FLEXIBLE_ARRAY_MEMBER within those structs (as you proposed in [1]) (as that should also prevent
> any possible wrong comments about where the array is located).
> 
> What do you think?

As mentioned above, It looks to me that making use of a FLEXIBLE_ARRAY_MEMBER is a good choice.
So, please find attached v35 making use of a FLEXIBLE_ARRAY_MEMBER in xl_hash_vacuum_one_page and gistxlogDelete (your 2 repros are not failing anymore).
I've also added a few words in the commit message in 0001 about it.

So, we end up with:

(gdb) ptype /o struct xl_hash_vacuum_one_page
/* offset      |    size */  type = struct xl_hash_vacuum_one_page {
/*      0      |       4 */    TransactionId snapshotConflictHorizon;
/*      4      |       4 */    int ntuples;
/*      8      |       1 */    _Bool isCatalogRel;
/* XXX  1-byte hole      */
/*     10      |       0 */    OffsetNumber offsets[];
/* XXX  2-byte padding   */

                                /* total size (bytes):   12 */
                              }

(gdb) ptype /o struct gistxlogDelete
/* offset      |    size */  type = struct gistxlogDelete {
/*      0      |       4 */    TransactionId snapshotConflictHorizon;
/*      4      |       2 */    uint16 ntodelete;
/*      6      |       1 */    _Bool isCatalogRel;
/* XXX  1-byte hole      */
/*      8      |       0 */    OffsetNumber offsets[];

                                /* total size (bytes):    8 */
                              }

While looking at it, I've a question: xl_hash_vacuum_one_page.ntuples is an int, do you see any reason why it is not an uint16? (we would get rid of 4 bytes in the struct).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com