Re: Minimal logical decoding on standbys

On Wed, Dec 12, 2018 at 3:41 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> I don't like the approach of managing the catalog horizon via those
> periodically logged catalog xmin announcements. I think we instead
> should build ontop of the records we already have and use to compute
> snapshot conflicts. As of HEAD we don't know whether such tables are
> catalog tables, but that's just a bool that we need to include in the
> records, a basically immeasurable overhead given the size of those
> records.

To me, this paragraph appears to say that you don't like Craig's
approach without quite explaining why you don't like it. Could you be
a bit more explicit about that?

> I also don't think we should actually enforce hot_standby_feedback being
> enabled - there's use-cases where that's not convenient, and it's not
> bullet proof anyway (can be enabled/disabled without using logical
> decoding inbetween). I think when there's a conflict we should have the
> HINT mention that hs_feedback can be used to prevent such conflicts,
> that ought to be enough.

If we can make that work, +1 from me.

> I'm wondering if it's time to move the latestRemovedXid computation for
> this type of record to the primary - it's likely to be cheaper there and
> avoids this kind of complication. Secondarily, it'd have the advantage
> of making pluggable storage integration easier - there we have the
> problem that we don't know which type of relation we're dealing with
> during recovery, so such lookups make pluggability harder (zheap just
> adds extra flags to signal that, but that's not extensible).

That doesn't look trivial. It seems like _bt_delitems_delete() would
need to get an array of XIDs, but that gets called from
_bt_vacuum_one_page(), which doesn't have that information available.
It doesn't look like there is a particularly cheap way of getting it,
either. What do you have in mind?

> Another alternative would be to just prevent such index deletions for
> catalog tables when wal_level = logical.

That doesn't sound like a very nice idea.

> If we were to go with this approach, there'd be at least the following
> tasks:
> - adapt tests from [2]

OK.

> - enforce hot-standby to be enabled on the standby when logical slots
> are created, and at startup if a logical slot exists

Why do we need this?

> - fix issue around btree_xlog_delete_get_latestRemovedXid etc mentioned
> above.

OK.

> - Have a nicer conflict handling than what I implemented here. Craig's
> approach deleted the slots, but I'm not sure I like that. Blocking
> seems more appropriately here, after all it's likely that the
> replication topology would be broken afterwards.

I guess the viable options are approximately -- (1) drop the slot, (2)
advance the slot, (3) mark the slot as "failed" but leave it in
existence as a tombstone, (4) wait until something changes. I like
(3) better than (1). (4) seems pretty unfortunate unless there's some
other system for having the slot advance automatically. Seems like a
way for replication to hang indefinitely without anybody understanding
why it's happened (or, maybe, noticing).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company