Re: [PATCH 10/16] Introduce the concept that wal has a 'origin' node

On Tuesday, June 19, 2012 11:46:56 PM Robert Haas wrote:
> On Tue, Jun 19, 2012 at 3:18 PM, Andres Freund <andres@2ndquadrant.com> 
wrote:
> > More seriously: Even if we don't put MM in core I think putting the basis
> > for it in core so that somebody can build such a solution reusing the
> > existing infrastructure is a sensible idea. Imo the only thing that
> > requires explicit support which is hard to add outside of core is
> > prevention of loops (aka this patch). Everything else should be doable
> > reusing the hopefully modular pieces.
> I don't think prevention of loops is hard to do outside of core
> either, unless you insist on tying "logical" replication so closely to
> WAL that anyone doing MMR is necessarily getting the change stream
> from WAL.  In fact, I'd go so far as to say that the ONLY part of this
> that's hard to do outside of core is change extraction.  Even
> low-level apply can be implemented as a loadable module.
I definitely agree that low-level apply is possible as a module. Sure change 
extraction needs core support but I was talking about what you need to 
implement it reusing the "plain" logical support...

What I do not understand is how you want to prevent loops in a simple manner 
without in core support:

A generates a HEAP_INSERT record. Gets decoded into the lcr stream as a INSERT 
action.
B reads the lcr stream from A and applies the changes. A new HEAP_INSERT 
record. Gets decoded into the lcr stream as a INSERT action.
A reads the lcr stream from B and ???

At this point you need to prevent a loop. If you have the information where a 
change originally happened (xl_origin_id = A in this case) you can have the 
simple filter on A which ignores change records if lcr_origin_id == 
local_replication_origin_id).


> >> Right.  If we decide we need this, and if we did decide to conflate
> >> the WAL stream, both of which I disagree with as noted above, then we
> >> still don't need it on every record.  It would probably be sufficient
> >> for local transactions to do nothing at all (and we can implicitly
> >> assume that they have master node ID = local node ID) and transactions
> >> which are replaying remote changes to emit one record per XID per
> >> checkpoint cycle containing the remote node ID.
> > 
> > Youve gone from a pretty trivial 150 line patch without any runtime/space
> > overhead to something *considerably* more complex in that case though.
> > 
> > You suddently need to have relatively complex logic to remember which
> > information you got for a certain xid (and forget that information
> > afterwards) and whether you already logged that xid and you need to have
> > to decide about logging that information at multiple places.
> You need a backend-local hash table inside the wal reader process, and
> that hash table needs to map XIDs to node IDs.  And you occasionally
> need to prune it, so that it doesn't eat too much memory.  None of
> that sounds very hard.
Its not very hard. Its just more complex than what I propose(d).

> > Btw, what do you mean with "conflating" the stream? I don't really see
> > that being proposed.
> It seems to me that you are intent on using the WAL stream as the
> logical change stream.  I think that's a bad design.  Instead, you
> should extract changes from WAL and then ship them around in a format
> that is specific to logical replication.
No, I don't want that. I think we will need some different format once we have 
agreed how changeset extraction works.

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