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.ga15...@awork2.anarazel.de
is renaming from
http://www.postgresql.org/message-id/20131008194758.gb3718...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
