From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: logical changeset generation v6.2 |
Date: | 2013-10-10 23:11:42 |
Message-ID: | 20131010231142.GC3924560@alap2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2013-10-10 23:30:04 | Re: dynamic shared memory: wherein I am punished for good intentions |
Previous Message | Bruce Momjian | 2013-10-10 22:58:21 | Re: Auto-tuning work_mem and maintenance_work_mem |