Re: [PATCH] Logical decoding support for sequence advances

On 15 December 2015 at 20:17, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>
> I think this is quite the wrong approach. You're calling the logical
> decoding callback directly from decode.c, circumventing
> reorderbuffer.c. While you don't need the actual reordering, you *do*
> need the snapshot integration.

Yeah. I thought I could get away without it because I could use
Form_pg_sequence.sequence_name to bypass any catalog lookups at all, but
per upthread that field should be ignored and can't be used.

> As you since noticed you can't just
> look into the sequence tuple and be happy with that, you need to do
> pg_class lookups et al. Furhtermore callbacks can validly expect to have
> a snapshot set.
>

Good point. Just because I don't need one doesn't mean others won't, and
all the other callbacks do.

I'll have a read over reorderbuffer.c and see if I can integrate this
properly.

> At the very least what you need to do is to SetupHistoricSnapshot(),
> then lookup the actual identity of the sequence using
> RelidByRelfilenode() and only then you can call the callback.
>

Yeah. That means it's safe for the callback to do a syscache lookup for the
sequence name by oid.

> > Needed to make logical replication based failover possible.
>
> While it'll make it easier, I think it's certainly quite possible to do
> so without this feature if you accept some sane restrictions. If you
> e.g. define to only handle serial columns (i.e. sequences that
> used/owned by exactly one table & column), you can do a 'good enough'
> emulation the output plugin.
>
> Take something like BDR's bdr_relcache.c (something which you're going
> to end up needing for any nontrivial replication solution). Everytime
> you build a cache entry you look up whether there's an owned by
> sequence. When decoding an insert or update to that relation, also emit
> an 'increase this sequence to at least XXX' message.
>

Eh. I don't think it's good enough for failover really. Too much of a
foot-gun risk for my taste. We've both seen the weird things users do and
the way nobody reads the manual or understands the limitations (global DDL
lock anybody?) and I really want to make things work right by default with
minimal caveats.

I'm not against working around it, and we'll need to for 9.4/9.5, but I'm
trying to get something more solid into 9.6.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services