Re: logical changeset generation v6.2

Hi Robert,

On 2013-10-09 14:49:46 -0400, Robert Haas wrote:
> I spent some time looking at the sample plugin (patch 9/12). Here are
> some review comments:
>
> - I think that the decoding plugin interface should work more like the
> foreign data wrapper interface. Instead of using pg_dlsym to look up
> fixed names, I think there should be a struct of function pointers
> that gets filled in and registered somehow.

You mean something like CREATE OUTPUT PLUGIN registering a function with
an INTERNAL return value returning a filled struct? I thought about
that, but it seemed more complex. Happy to change it though if it's
preferred.

> - pg_decode_init() only warns when it encounters an unknown option.
> An error seems more appropriate.

Fine with me. I think I just made it a warning because I wanted to
experiment with options.

> - Still wondering how we'll use this from a bgworker.

Simplified code to consume data:

LogicalDecodingReAcquireSlot(NameStr(*name));
ctx = CreateLogicalDecodingContext(MyLogicalDecodingSlot, false /* not initial call */,
MyLogicalDecodingSlot->confirmed_flush,
options,
logical_read_local_xlog_page,
LogicalOutputPrepareWrite,
LogicalOutputWrite);
...
while (true)
{
XLogRecord *record;
char *errm = NULL;

record = XLogReadRecord(ctx->reader, startptr, &errm);
...
DecodeRecordIntoReorderBuffer(ctx, &buf);
}

/* at the end or better ever commit or such */
LogicalConfirmReceivedLocation(/* whatever you consumed */);

LogicalDecodingReleaseSlot();

> - The output format doesn't look very machine-parseable. I really
> think we ought to provide something that is. Maybe a CSV-like format,
> or maybe something else, but I don't see why someone who wants to do
> change logging should be forced to write and install C code. If
> something like Bucardo can run on an unmodified system and extract
> change-sets this way without needing a .so file, that's going to be a
> huge win for usability.

We can change the current format but I really see little to no chance of
agreeing on a replication format that's serviceable to several solutions
short term. Once we've gained some experience - maybe even this cycle -
that might be different.

> More generally on this patch set, if I'm going to be committing any of
> this, I'd prefer to start with what is currently patches 3 and 4, once
> we reach agreement on those.

Sounds like a reasonable start.

> Are we hoping to get any of this committed for this CF? If so, let's
> make a plan to get that done; time is short. If not, let's update the
> CF app accordingly.

I'd really like to do so. I am travelling atm, but I will be back
tomorrow evening and will push an updated patch this weekend. The issue
I know of in the latest patches at
http://www.postgresql.org/message-id/20131007133232.GA15202@awork2.anarazel.de
is renaming from http://www.postgresql.org/message-id/20131008194758.GB3718183@alap2.anarazel.de

Do you know of anything else in the patches you're referring to?

Thanks,

Andres Freund

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