Horton

Re: Minimal logical decoding on standbys

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Jeff Davis <pgsql@j-davis.com>, Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.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: 2023-04-07T02:18:30Z
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.

Hi,

TBH, I don't like the state of 0001 much. I'm working on polishing it now.

A lot of the new functions in slot.h don't seem right to me:
- ObsoleteSlotIsInvalid() - isn't an obsolete slot by definition invalid?
- Why does ObsoleteSlotIsInvalid() sometime check invalidated_at and sometimes
  not?
- DoNotInvalidateSlot() seems too generic a name for a function exposed to the
  outside of slot.c
- TransactionIdIsValidPrecedesOrEquals() shouldn't be defined in slot.h -
  also, it's not actually clear what semantics it's trying to have.
- there's no commonality in naming between the functions used to test if a
  slot needs to be invalidated (SlotIsFreshEnough() vs
  LogicalSlotIsNotConflicting()).

Leaving naming etc aside, most of these don't seem to belong in slot.h, but
should just be in slot.c - there aren't conceivable users from outside slot.c.


Independent of this patch: What's the point of invalidated_at? The only reads
of it are done like
                invalidated = (!XLogRecPtrIsInvalid(s->data.invalidated_at) &&
                                           XLogRecPtrIsInvalid(s->data.restart_lsn));
i.e. the actual LSN is not used.

ISTM that we should just have it be a boolean, and that it should be used by
the different kinds of invalidating a slot.

Greetings,

Andres Freund