Re: Minimal logical decoding on standbys

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Thomas Munro <thomas.munro@gmail.com>, 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>, Melanie Plageman <melanieplageman@gmail.com>
Date: 2023-04-02T03:42:44Z
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,

On 2023-03-31 12:45:51 +0200, Drouvot, Bertrand wrote:
> On 3/31/23 6:33 AM, Andres Freund wrote:
> > Hi,
> > 
> > On 2023-03-30 18:23:41 +0200, Drouvot, Bertrand wrote:
> > > On 3/30/23 9:04 AM, Andres Freund wrote:
> > > > I think this commit is ready to go. Unless somebody thinks differently, I
> > > > think I might push it tomorrow.
> > > 
> > > Great! Once done, I'll submit a new patch so that GlobalVisTestFor() can make
> > > use of the heap relation in vacuumRedirectAndPlaceholder() (which will be possible
> > > once 0001 is committed).
> > 
> > Unfortunately I did find an issue doing a pre-commit review of the patch.
> > 
> > The patch adds VISIBILITYMAP_IS_CATALOG_REL to xl_heap_visible.flags - but it
> > does not remove the bit before calling visibilitymap_set().
> > 
> > This ends up corrupting the visibilitymap, because the we'll set a bit for
> > another page.
> > 
> 
> Oh I see, I did not think about that (not enough experience in the VM area).
> Nice catch and thanks for pointing out!

I pushed a commit just adding an assertion that only valid bits are passed in.


> > I'm also thinking of splitting the patch into two. One patch to pass down the
> > heap relation into the new places, and another for the rest.
> 
> I think that makes sense. I don't know how far you've work on the split but please
> find attached V54 doing such a split + implementing your VISIBILITYMAP_XLOG_VALID_BITS
> suggestion.

I pushed the pass-the-relation part.  I removed an include of catalog.h that
was in the patch - I suspect it might have slipped in there from a later patch
in the series...

I was a bit bothered by using 'heap' instead of 'table' in so many places
(eventually we imo should standardize on the latter), but looking around the
changed places, heap was used for things like buffers etc. So I left it at
heap.

Glad we split 0001 - the rest is a lot easier to review.

Greetings,

Andres Freund