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

On 20 June 2012 20:37, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
>> Heikki Linnakangas  wrote:
>
>> I don't like the idea of adding the origin id to the record header.
>> It's only required in some occasions, and on some record types.
>
> Right.

Wrong, as explained.

>> And I'm worried it might not even be enough in more complicated
>> scenarios.
>>
>> Perhaps we need a more generic WAL record annotation system, where
>> a plugin can tack arbitrary information to WAL records. The extra
>> information could be stored in the WAL record after the rmgr
>> payload, similar to how backup blocks are stored. WAL replay could
>> just ignore the annotations, but a replication system could use it
>> to store the origin id or whatever extra information it needs.
>
> Not only would that handle absolute versus relative updates and
> origin id, but application frameworks could take advantage of such a
> system for passing transaction metadata.  I've held back on one
> concern so far that I'll bring up now because this suggestion would
> address it nicely.
>
> Our current trigger-driven logical replication includes a summary
> which includes transaction run time, commit time, the transaction
> type identifier, the source code line from which that transaction was
> invoked, the user ID with which the user connected to the application
> (which isn't the same as the database login), etc.  Being able to
> "decorate" a database transaction with arbitrary (from the DBMS POV)
> metadata would be very valuable.  In fact, our shop can't maintain
> the current level of capabilities without *some* way to associate
> such information with a transaction.

> I think that using up the only unused space in the fixed header to
> capture one piece of the transaction metadata needed for logical
> replication, and that only in some configurations, is short-sighted.
> If we solve the general problem of transaction metadata, this one
> specific case will fall out of that.

The proposal now includes flag bits that would allow the addition of a
variable length header, should that ever become necessary. So the
unused space in the fixed header is not being "used up" as you say. In
any case, the fixed header still has 4 wasted bytes on 64bit systems
even after the patch is applied. So this claim of short sightedness is
just plain wrong.

It isn't true that this is needed only for some configurations of
multi-master, per discussion.

This is not transaction metadata, it is WAL record metadata required
for multi-master replication, see later point.

We need to add information to every WAL record that is used as the
source for generating LCRs. It is also possible to add this to HEAP
and HEAP2 records, but doing that *will* bloat the WAL stream, whereas
using the *currently wasted* bytes on a WAL record header does *not*
bloat the WAL stream.

> I think removing origin ID from this patch and submitting a separate
> patch for a generalized transaction metadata system is the sensible
> way to go.

We already have a very flexible WAL system for recording data of
interest to various resource managers. If you wish to annotate a
transaction, you can either generate a new kind of WAL record or you
can enhance a commit record. There are already unused flag bits on
commit records for just such a purpose.

XLOG_NOOP records can already be generated by your application if you
wish to inject additional metadata to the WAL stream. So no changes
are required for you to implement the generalised transaction metadata
scheme you say you require.

Not sure how or why that relates to requirements for multi-master.

Please note that I've suggested review changes to Andres' work myself.

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