Re: [HACKERS] Changeset Extraction v7.5
On 2014-02-11 11:22:24 -0500, Robert Haas wrote: > + context = AllocSetContextCreate(CurrentMemoryContext, > + > "Changeset Extraction Context", > + > ALLOCSET_DEFAULT_MINSIZE, > + > ALLOCSET_DEFAULT_INITSIZE, > + > ALLOCSET_DEFAULT_MAXSIZE); > > I have my doubts about whether it's wise to make this the child of > CurrentMemoryContext. Certainly, if we do that, then expectations > around what that context is need to be documented. Short-lived > contexts are presumably unsuitable. Well, it depends on the type of usage. In the walsender, yes, it needs to be a longliving context. Not so much in the SQL case, inside the SRF we spill all the data into a tuplestore after which we are done. I don't see which context would be more suitable as a default parent; it used to be TopMemoryContext but that requires pointless cleanup routines to handle errors... So I think documenting the requirement is the best way? I'm working on the other comments, pushing individual changes to git. Will send a new version onlist once I'm through. Greetings, 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
Re: [HACKERS] Changeset Extraction v7.5
Hi! On 2014-02-11 11:22:24 -0500, Robert Haas wrote: > + * contents of records in here xexcept turning them into a more usable > > Typo. > > + /* > +* XXX: There doesn't seem to be a usecase for > decoding > +* HEAP_NEWPAGE's. Its only used in various > indexam's and CLUSTER, > +* neither of which should be relevant for the logical > +* changestream. > +*/ > > There's a level of uncertainty here that doesn't seem consistent with > calling this a finished patch. It's also not a complete list of > places where log_newpage() is called, but frankly I don't think that > should be the aim of this comment. The only relevant question is > whether we ever use XLOG_HEAP_NEWPAGE to log heap changes that are > relevant to logical replication. I think we don't. You're right, we currently don't. I guess we should add a comment to log_newpage()/buffer to make sure that's not violated in future code, that seems like the only place that's somewhat likely to be read? > Decoding PREPARE might > enable future code to prepare each locally prepared transaction on the > remote side before doing a COMMIT TRANSACTION locally, allowing for > logical synchronous replication." We do support logical synchronous replication over the walsender interface, what it would allow us to do would be some form of 2pc... I'll adapt your version. > + /* > +* If the record wasn't part of a transaction, > it will not have > +* caused invalidations and thus isn't > important when building > +* snapshots. If it was part of a transaction, > that transaction > +* just performed DDL because those are the > only codepaths using > +* inplace updates. > +*/ > > Under what circumstances do we issue in-place updates not associated > with a transaction? And under what circumstances do we issue in-place > updates that ARE associated with a transaction? vacuum updates relfrozenxid outside a transaction, e.g. create/drop index, analyze inside one. > +/* > + * Get the data from the various forms of commit records and pass it > + * on to snapbuild.c and reorderbuffer.c > + */ > > This is a lousy comment. I suggest something like: "Currently, each > transaction is decoded only once it commit, so the arrival of a commit > record means that we can now decode the changes made by this toplevel > transaction and all of its committed subtransactions, unless we have > to skip it because the replication system isn't fully initialized yet. > Whether decoding the transaction or not, we must take note of any > invalidations it issues, as those will affect the snapshot used for > decoding of *other* transactions." > > +/* > + * Get the data from the various forms of abort records and pass it on to > + * snapbuild.c and reorderbuffer.c > + */ > > Suggest: "When a transaction abort is detected, we throw away any data > we've stashed away for possible future decoding of that transaction. > Knowledge of the abort may also help us establish our initial snapshot > when logical decoding is first initiated." Hm, those should go into reorderbuffer.c instead, the interesting part of DecodeCommit/DecodeAbort is just to centralize the handling of the various forms of commit/abort records. decode.c really shouldn't need to be changed much when we start to optionally support streaming out changes immediately. > + /* register output plugin name with slot */ > + strncpy(NameStr(MyReplicationSlot->data.plugin), plugin, > + NAMEDATALEN); > + NameStr(MyReplicationSlot->data.plugin)[NAMEDATALEN - 1] = > '\0'; > > Hmm. Shouldn't this be delegated to something in slot.c? Why don't > we need the lock? I wasn't sure, we can place it there, but it doesn't really need to know about these details either. Lockingwise I don't see it needing more, nobody but the slot that has it acquired is interested in it. > Why don't we need to fsync() the change? I've since pushed a patch that does the fsyncing. Not doing so was part of a rebase screwup. > + /* > +* FIXME: we're going to have to do something more > intelligent about > +* timelines on standby's. Use readTimeLineHistory() and > +* tliOfPointInHistory() to get the proper LSN? > +*/ > > So what's the plan for that? Prohibit decoding on the standby for now. Not sure how to deal with the relevant code, leave it there, #ifdef it out, remove it? > + /* > +* XXX: It'd be way nicer to be able to use the > walsender waiting logic > +* here, but that's not available in all environments. > +*/ > > I don't understand
Re: [HACKERS] Changeset Extraction v7.5
On Fri, Feb 7, 2014 at 2:35 PM, Andres Freund wrote: > attached you can find the next version of the patchset. As usual, I'm going to be reviewing patch 1. The definition of "patch 1" has changed quite a few times over the past year, but that's usually the one I'm reviewing. + * contents of records in here xexcept turning them into a more usable Typo. + /* +* XXX: There doesn't seem to be a usecase for decoding +* HEAP_NEWPAGE's. Its only used in various indexam's and CLUSTER, +* neither of which should be relevant for the logical +* changestream. +*/ There's a level of uncertainty here that doesn't seem consistent with calling this a finished patch. It's also not a complete list of places where log_newpage() is called, but frankly I don't think that should be the aim of this comment. The only relevant question is whether we ever use XLOG_HEAP_NEWPAGE to log heap changes that are relevant to logical replication. I think we don't. + /* FIXME: skip if wrong db? */ It's time to fish or cut bait. + /* +* XXX: As a future feature, we could replay the transaction and +* prepare it as well, allowing for 2PC via logical decoding. +*/ Let's try to avoid using XXX (or FIXME) for things that really mean TODO. I think this comment deserves to be expanded a bit, too. Maybe something like: "Right now, logical decoding ignores PREPARE TRANSACTION and simply decodes the subsequent COMMIT TRANSACTION or ROLLBACK TRANSACTION just as it would a regular COMMIT or ROLLBACK. In the future, we might want to change this. Decoding PREPARE might enable future code to prepare each locally prepared transaction on the remote side before doing a COMMIT TRANSACTION locally, allowing for logical synchronous replication." + /* +* If the record wasn't part of a transaction, it will not have +* caused invalidations and thus isn't important when building +* snapshots. If it was part of a transaction, that transaction +* just performed DDL because those are the only codepaths using +* inplace updates. +*/ Under what circumstances do we issue in-place updates not associated with a transaction? And under what circumstances do we issue in-place updates that ARE associated with a transaction? +* XXX: At some point we might want to execute the transaction's The XXX again seems needless; the comment is fine as it stands. + /* +* Abort all transactions that we keep track of that are older +* than ->oldestRunningXid. This is the most convenient spot I think writing ->membername is poor commenting style. Just leave out the arrow, or write "the WAL record's oldestRunningXid." +/* + * Get the data from the various forms of commit records and pass it + * on to snapbuild.c and reorderbuffer.c + */ This is a lousy comment. I suggest something like: "Currently, each transaction is decoded only once it commit, so the arrival of a commit record means that we can now decode the changes made by this toplevel transaction and all of its committed subtransactions, unless we have to skip it because the replication system isn't fully initialized yet. Whether decoding the transaction or not, we must take note of any invalidations it issues, as those will affect the snapshot used for decoding of *other* transactions." +/* + * Get the data from the various forms of abort records and pass it on to + * snapbuild.c and reorderbuffer.c + */ Suggest: "When a transaction abort is detected, we throw away any data we've stashed away for possible future decoding of that transaction. Knowledge of the abort may also help us establish our initial snapshot when logical decoding is first initiated." +/* + * Set the xmin required for decoding snapshots for the specific decoding + * slot. + */ +void +IncreaseLogicalXminForSlot(XLogRecPtr lsn, TransactionId xmin) I'm thinking this and everything that follows, up through LogicalDecodingCountDBSlots, probably should be moved to slot.c. + /* XXX: Add the current LSN? */ +1. + /* shorter lines... */ + slot = MyReplicationSlot; If you're going to do this, which seems like it's probably a good idea, do it at the top of the function and use it all the way through instead of doing it in the middle. + if (MyReplicationSlot == NULL) + elog(ERROR, "need a current slot"); + + if (is_init && start_lsn != InvalidXLogRecPtr) + elog(ERROR, "Cannot INIT_LOGICAL_REPLICATION at a specified LSN"); + + if (is_init && p
Re: [HACKERS] Changeset Extraction v7.5
On 9 February 2014 01:06, Andres Freund wrote:
> On 2014-02-09 00:49:31 +, Thom Brown wrote:
>> # ALTER TABLE test ADD COLUMN a decimal DEFAULT 2.22;
>> ALTER TABLE
>>
>> # ALTER TABLE test ADD COLUMN b json DEFAULT '{"a":[1,2,3],"b":[4,5,6]}';
>> ALTER TABLE
>>
>> The output generated by those last 2 statements is:
>>
>> BEGIN 891
>> table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
>> table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
>> table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
>> COMMIT 891
>> BEGIN 892
>> table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
>> b[json]:{"a":[1,2,3],"b":[4,5,6]}
>> table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
>> b[json]:{"a":[1,2,3],"b":[4,5,6]}
>> table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
>> b[json]:{"a":[1,2,3],"b":[4,5,6]}
>> COMMIT 892
>>
>> This is showing inserts into the temp table as part of the operation.
>> Is that sufficient?
>
> I think it's a good thing for now. We don't have support for DDL
> replication so it's not yet that interesting, but having the new values
> allows to safely handle things like DEFAULTs that produce
> nondeterministic data.
> What do you think?
Okay, I'm just checking. If it's expected behaviour to you, it's good
enough for me.
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 2014-02-09 00:49:31 +, Thom Brown wrote:
> # ALTER TABLE test ADD COLUMN a decimal DEFAULT 2.22;
> ALTER TABLE
>
> # ALTER TABLE test ADD COLUMN b json DEFAULT '{"a":[1,2,3],"b":[4,5,6]}';
> ALTER TABLE
>
> The output generated by those last 2 statements is:
>
> BEGIN 891
> table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
> table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
> table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
> COMMIT 891
> BEGIN 892
> table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
> b[json]:{"a":[1,2,3],"b":[4,5,6]}
> table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
> b[json]:{"a":[1,2,3],"b":[4,5,6]}
> table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
> b[json]:{"a":[1,2,3],"b":[4,5,6]}
> COMMIT 892
>
> This is showing inserts into the temp table as part of the operation.
> Is that sufficient?
I think it's a good thing for now. We don't have support for DDL
replication so it's not yet that interesting, but having the new values
allows to safely handle things like DEFAULTs that produce
nondeterministic data.
What do you think?
Greetings,
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
Re: [HACKERS] Changeset Extraction v7.5
On 8 February 2014 23:08, Andres Freund wrote:
> On 2014-02-08 22:58:35 +, Thom Brown wrote:
>> Another question: in order for logical decoding/replication to be
>> useful, presumably one would need a primary key on every table? It's
>> just I haven't seen this mentioned on the changeset extraction page in
>> the docs.
>
> Hm, that's a good point. 07cacba983ef79be4a84fcd0e0ca3b5fcb85dd65 added
> configurability for that, but there at least should be a link to
> http://www.postgresql.org/docs/devel/static/sql-altertable.html with
> some additional words.
# CREATE TABLE test (id serial primary key, val int);
CREATE TABLE
# INSERT INTO test (val) SELECT generate_series(1,3);
INSERT 0 3
# ALTER TABLE test ADD COLUMN a decimal DEFAULT 2.22;
ALTER TABLE
# ALTER TABLE test ADD COLUMN b json DEFAULT '{"a":[1,2,3],"b":[4,5,6]}';
ALTER TABLE
The output generated by those last 2 statements is:
BEGIN 891
table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
COMMIT 891
BEGIN 892
table "pg_temp_16552": INSERT: id[int4]:1 val[int4]:1 a[numeric]:2.22
b[json]:{"a":[1,2,3],"b":[4,5,6]}
table "pg_temp_16552": INSERT: id[int4]:2 val[int4]:2 a[numeric]:2.22
b[json]:{"a":[1,2,3],"b":[4,5,6]}
table "pg_temp_16552": INSERT: id[int4]:3 val[int4]:3 a[numeric]:2.22
b[json]:{"a":[1,2,3],"b":[4,5,6]}
COMMIT 892
This is showing inserts into the temp table as part of the operation.
Is that sufficient?
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 2014-02-08 22:58:35 +, Thom Brown wrote: > Another question: in order for logical decoding/replication to be > useful, presumably one would need a primary key on every table? It's > just I haven't seen this mentioned on the changeset extraction page in > the docs. Hm, that's a good point. 07cacba983ef79be4a84fcd0e0ca3b5fcb85dd65 added configurability for that, but there at least should be a link to http://www.postgresql.org/docs/devel/static/sql-altertable.html with some additional words. Greetings, 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
Re: [HACKERS] Changeset Extraction v7.5
On 8 February 2014 22:47, Andres Freund wrote:
> Hi Thom,
>
> On 2014-02-08 22:26:59 +, Thom Brown wrote:
>> Got a question about ranges and arrays usage with timestamps... why
>> are quotes added to these?
>>
>> timestamptz (no quotes with input or output):
>> table "a": INSERT: moo[timestamptz]:2014-02-08 22:09:33+00
>>
>> tstzrange (no quotes with input, but quotes with output):
>> table "b": INSERT: moo[tstzrange]:["2014-02-08
>> 13:45:22+00","2014-02-08 14:45:42+00")
>>
>> timestamptz[] (no quotes with input, but quotes with output):
>> table "c": INSERT: moo[_timestamptz]:{"2010-01-01
>> 13:45:22+00","2010-01-03 14:45:42+00"}
>>
>> tstzrange[] (one set of quotes with input, two sets of quotes with
>> output, one set of which are escaped):
>> table "d": INSERT: moo[_tstzrange]:{"(\"2014-02-08
>> 13:45:22+00\",\"2014-02-08 13:45:42+00\"]","[\"2014-02-07
>> 10:12:19+00\",\"2014-02-07 13:51:16+00\"]"}
>
> The test_decoding output plugin just uses the default text output
> functions for all types, other plugins could do differently. I.e. in all
> these cases a SELECT, COPY, pg_dump will also include those quotes.
> E.g.
> postgres=# SELECT ARRAY[tstzrange(NOW(), NOW() + interval '1 day')];
> will format it's output similarly:
> {"[\"2014-02-08 23:44:19.82007+01\",\"2014-02-09 23:44:19.82007+01\")"}
> (1 row)
>
> Does that answer make sense?
Ah, okay. Thanks.
Another question: in order for logical decoding/replication to be
useful, presumably one would need a primary key on every table? It's
just I haven't seen this mentioned on the changeset extraction page in
the docs.
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
Hi Thom,
On 2014-02-08 22:26:59 +, Thom Brown wrote:
> Got a question about ranges and arrays usage with timestamps... why
> are quotes added to these?
>
> timestamptz (no quotes with input or output):
> table "a": INSERT: moo[timestamptz]:2014-02-08 22:09:33+00
>
> tstzrange (no quotes with input, but quotes with output):
> table "b": INSERT: moo[tstzrange]:["2014-02-08
> 13:45:22+00","2014-02-08 14:45:42+00")
>
> timestamptz[] (no quotes with input, but quotes with output):
> table "c": INSERT: moo[_timestamptz]:{"2010-01-01
> 13:45:22+00","2010-01-03 14:45:42+00"}
>
> tstzrange[] (one set of quotes with input, two sets of quotes with
> output, one set of which are escaped):
> table "d": INSERT: moo[_tstzrange]:{"(\"2014-02-08
> 13:45:22+00\",\"2014-02-08 13:45:42+00\"]","[\"2014-02-07
> 10:12:19+00\",\"2014-02-07 13:51:16+00\"]"}
The test_decoding output plugin just uses the default text output
functions for all types, other plugins could do differently. I.e. in all
these cases a SELECT, COPY, pg_dump will also include those quotes.
E.g.
postgres=# SELECT ARRAY[tstzrange(NOW(), NOW() + interval '1 day')];
will format it's output similarly:
{"[\"2014-02-08 23:44:19.82007+01\",\"2014-02-09 23:44:19.82007+01\")"}
(1 row)
Does that answer make sense?
Greetings,
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
Re: [HACKERS] Changeset Extraction v7.5
On 8 February 2014 21:25, Andres Freund wrote:
> On 2014-02-08 21:07:03 +, Thom Brown wrote:
>> > I'll continue to play around with the feature.
>>
>> Next issue. Firstly, an out-of-date example:
>>
>> doc/src/sgml/changesetextraction.sgml
>>
>> pg_recvlogical --slot test --init -d testdb
>>
>> There's no option --init. I think this is supposed to be --create.
>
> Fixed. It used to be --init, but that has changed. Thanks.
>
>> $ pg_recvlogical --slot test --create -d testdb
>> pg_recvlogical: could not send replication command
>> "CREATE_REPLICATION_SLOT "test" LOGICAL "test_decoding"": extraneous
>> data in "T" message
>
> Gah. Another merge issue. Fixed. We really need to have infrastructure
> for testing binaries...
Thanks, no issue with that now.
Got a question about ranges and arrays usage with timestamps... why
are quotes added to these?
timestamptz (no quotes with input or output):
table "a": INSERT: moo[timestamptz]:2014-02-08 22:09:33+00
tstzrange (no quotes with input, but quotes with output):
table "b": INSERT: moo[tstzrange]:["2014-02-08
13:45:22+00","2014-02-08 14:45:42+00")
timestamptz[] (no quotes with input, but quotes with output):
table "c": INSERT: moo[_timestamptz]:{"2010-01-01
13:45:22+00","2010-01-03 14:45:42+00"}
tstzrange[] (one set of quotes with input, two sets of quotes with
output, one set of which are escaped):
table "d": INSERT: moo[_tstzrange]:{"(\"2014-02-08
13:45:22+00\",\"2014-02-08 13:45:42+00\"]","[\"2014-02-07
10:12:19+00\",\"2014-02-07 13:51:16+00\"]"}
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 2014-02-08 21:07:03 +, Thom Brown wrote: > > I'll continue to play around with the feature. > > Next issue. Firstly, an out-of-date example: > > doc/src/sgml/changesetextraction.sgml > > pg_recvlogical --slot test --init -d testdb > > There's no option --init. I think this is supposed to be --create. Fixed. It used to be --init, but that has changed. Thanks. > $ pg_recvlogical --slot test --create -d testdb > pg_recvlogical: could not send replication command > "CREATE_REPLICATION_SLOT "test" LOGICAL "test_decoding"": extraneous > data in "T" message Gah. Another merge issue. Fixed. We really need to have infrastructure for testing binaries... 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
Re: [HACKERS] Changeset Extraction v7.5
On 8 February 2014 19:35, Thom Brown wrote: > On 8 February 2014 17:52, Andres Freund wrote: >> Hi, >> >> Only got to this now, was a bit too tired and needed to catch up on some >> real-world stuff... >> >> On 2014-02-08 00:16:07 +, Thom Brown wrote: >>> On 7 February 2014 23:43, Andres Freund wrote: >>> > Thanks, that's a bug indeed. I have experimentally fixed the bug, not >>> > sure whether I like the fix yet, or not. >>> > >>> > I've already fixed two issues caused by the rebase onto >>> > 858ec11858a914d4c380971985709b6d6b7dd6fc. >>> > >>> > Is pushing to git sufficient for you, or shall I rebase and resend the >>> > series? >>> >>> Sure, push it to git, I'll add your remote repo and checkout that branch. >> >> Ok, I roughly went with my initial plan to fix this and I've added (and >> fixed) a regression for this. >> >> Pushed this and some other improvements to >> http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=summary >> branch xlog-decoding-rebasing-remapping > > This appears to be working now. Thanks. > > I'll continue to play around with the feature. Next issue. Firstly, an out-of-date example: doc/src/sgml/changesetextraction.sgml pg_recvlogical --slot test --init -d testdb There's no option --init. I think this is supposed to be --create. But also: $ pg_recvlogical --slot test --create -d testdb pg_recvlogical: could not send replication command "CREATE_REPLICATION_SLOT "test" LOGICAL "test_decoding"": extraneous data in "T" message But this seems to have created it anyway: # SELECT * FROM pg_replication_slots; slot_name |plugin | slot_type | datoid | database | active | xmin | catalog_xmin | restart_lsn ---+---+---++--++--+--+- test | test_decoding | logical | 16396 | testdb | t | | | 0/16E2030 (1 row) If I drop it and run the same command, the same message is emitted. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 8 February 2014 17:52, Andres Freund wrote: > Hi, > > Only got to this now, was a bit too tired and needed to catch up on some > real-world stuff... > > On 2014-02-08 00:16:07 +, Thom Brown wrote: >> On 7 February 2014 23:43, Andres Freund wrote: >> > Thanks, that's a bug indeed. I have experimentally fixed the bug, not >> > sure whether I like the fix yet, or not. >> > >> > I've already fixed two issues caused by the rebase onto >> > 858ec11858a914d4c380971985709b6d6b7dd6fc. >> > >> > Is pushing to git sufficient for you, or shall I rebase and resend the >> > series? >> >> Sure, push it to git, I'll add your remote repo and checkout that branch. > > Ok, I roughly went with my initial plan to fix this and I've added (and > fixed) a regression for this. > > Pushed this and some other improvements to > http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=summary > branch xlog-decoding-rebasing-remapping This appears to be working now. Thanks. I'll continue to play around with the feature. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
Hi, Only got to this now, was a bit too tired and needed to catch up on some real-world stuff... On 2014-02-08 00:16:07 +, Thom Brown wrote: > On 7 February 2014 23:43, Andres Freund wrote: > > Thanks, that's a bug indeed. I have experimentally fixed the bug, not > > sure whether I like the fix yet, or not. > > > > I've already fixed two issues caused by the rebase onto > > 858ec11858a914d4c380971985709b6d6b7dd6fc. > > > > Is pushing to git sufficient for you, or shall I rebase and resend the > > series? > > Sure, push it to git, I'll add your remote repo and checkout that branch. Ok, I roughly went with my initial plan to fix this and I've added (and fixed) a regression for this. Pushed this and some other improvements to http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=summary branch xlog-decoding-rebasing-remapping 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
Re: [HACKERS] Changeset Extraction v7.5
On 7 February 2014 23:43, Andres Freund wrote: > On 2014-02-07 20:58:14 +, Thom Brown wrote: >> On 7 February 2014 19:35, Andres Freund wrote: >> > 0004: wal_decoding: Documentation for replication slots and changeset >> > extraction >> >> The usage of pg_create_decoding_replication_slot does show the "(1 row)" >> line. >> >> The output of "SELECT * FROM pg_replication_slots;" is out-of-date. > > Thanks, refreshed. > >> There appears to be a column named "slot_name" and "slottype". Could >> one of these have or not have the underscore for consistency? > > That's luckily already fixed... > >> The example also shows output from pg_decoding_slot_get_changes after >> inserting 2 rows, but when I run the same example, there are no rows >> returned: > >> And running the transaction with inserts again, there's no output from >> that same function command. I always get an output from isolated >> INSERT statements. I should point out that in my .psqlrc file I have >> "\set ON_ERROR_ROLLBACK". If I use psql -X, this symptom no longer >> occurs, so I think the automatic savepoints are interfering, and the >> effect appears to be inconsistent. > > Thanks, that's a bug indeed. I have experimentally fixed the bug, not > sure whether I like the fix yet, or not. > > I've already fixed two issues caused by the rebase onto > 858ec11858a914d4c380971985709b6d6b7dd6fc. > > Is pushing to git sufficient for you, or shall I rebase and resend the > series? Sure, push it to git, I'll add your remote repo and checkout that branch. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 2014-02-07 20:58:14 +, Thom Brown wrote: > On 7 February 2014 19:35, Andres Freund wrote: > > 0004: wal_decoding: Documentation for replication slots and changeset > > extraction > > The usage of pg_create_decoding_replication_slot does show the "(1 row)" line. > > The output of "SELECT * FROM pg_replication_slots;" is out-of-date. Thanks, refreshed. > There appears to be a column named "slot_name" and "slottype". Could > one of these have or not have the underscore for consistency? That's luckily already fixed... > The example also shows output from pg_decoding_slot_get_changes after > inserting 2 rows, but when I run the same example, there are no rows > returned: > And running the transaction with inserts again, there's no output from > that same function command. I always get an output from isolated > INSERT statements. I should point out that in my .psqlrc file I have > "\set ON_ERROR_ROLLBACK". If I use psql -X, this symptom no longer > occurs, so I think the automatic savepoints are interfering, and the > effect appears to be inconsistent. Thanks, that's a bug indeed. I have experimentally fixed the bug, not sure whether I like the fix yet, or not. I've already fixed two issues caused by the rebase onto 858ec11858a914d4c380971985709b6d6b7dd6fc. Is pushing to git sufficient for you, or shall I rebase and resend the series? 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
Re: [HACKERS] Changeset Extraction v7.5
On Fri, February 7, 2014 22:29, Thom Brown wrote: > On 7 February 2014 21:28, Erik Rijkers wrote: >> On Fri, February 7, 2014 22:09, Thom Brown wrote: >> The example also shows output from pg_decoding_slot_get_changes after inserting 2 rows, but when I run the same example, there are no rows >> >> FWIW, works for me: > > Can you confirm you're running it with ON_ERROR_ROLLBACK set? > Ah, no, I missed that. You're right: with that, behaviour is the same here as you described. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 7 February 2014 21:28, Erik Rijkers wrote: > On Fri, February 7, 2014 22:09, Thom Brown wrote: > >>>The example also shows output from pg_decoding_slot_get_changes after >>>inserting 2 rows, but when I run the same example, there are no rows > > FWIW, works for me: Can you confirm you're running it with ON_ERROR_ROLLBACK set? -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On Fri, February 7, 2014 22:09, Thom Brown wrote:
>>The example also shows output from pg_decoding_slot_get_changes after
>>inserting 2 rows, but when I run the same example, there are no rows
FWIW, works for me:
testdb=# SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
'include-xids', '0');
location | xid | data
--+-+--
(0 rows)
testdb=# BEGIN; INSERT INTO data(data) VALUES('1'); INSERT INTO data(data)
VALUES('1'); COMMIT;
testdb=# SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
'include-xids', '0');
location | xid | data
---+--+
0/2B81ED0 | 1973 | BEGIN
0/2B823A8 | 1973 | table "data": INSERT: id[int4]:14 data[text]:1
0/2B823A8 | 1973 | table "data": INSERT: id[int4]:15 data[text]:1
0/2B823A8 | 1973 | COMMIT
(4 rows)
testdb=# SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
'include-xids', '0');
location | xid | data
--+-+--
(0 rows)
( output of "SELECT * FROM pg_replication_slots;" is, indeed, out-of-date.)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 7 February 2014 19:35, Andres Freund wrote:
> 0004: wal_decoding: Documentation for replication slots and changeset
> extraction
The usage of pg_create_decoding_replication_slot does show the "(1 row)" line.
The output of "SELECT * FROM pg_replication_slots;" is out-of-date.
There appears to be a column named "slot_name" and "slottype". Could
one of these have or not have the underscore for consistency?
The example also shows output from pg_decoding_slot_get_changes after
inserting 2 rows, but when I run the same example, there are no rows
returned:
# BEGIN;
BEGIN
*# INSERT INTO data(data) VALUES('1');
INSERT 0 1
*# INSERT INTO data(data) VALUES('1');
INSERT 0 1
*# COMMIT;
COMMIT
# SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
'include-xids', '0');
location | xid | data
--+-+--
(0 rows)
I inserted a single row outside of a transaction, and got the expected
output. Then I ran the above again, and got an output, but an
unexpected one:
SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
'include-xids', '0');
location | xid | data
---+-+---
0/16C8B90 | 769 | BEGIN
0/16C8D50 | 769 | table "data": INSERT: id[int4]:3 data[text]:1
0/16C8D50 | 769 | COMMIT
(3 rows)
And running the transaction with inserts again, there's no output from
that same function command. I always get an output from isolated
INSERT statements. I should point out that in my .psqlrc file I have
"\set ON_ERROR_ROLLBACK". If I use psql -X, this symptom no longer
occurs, so I think the automatic savepoints are interfering, and the
effect appears to be inconsistent.
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On 7 February 2014 21:04, Andres Freund wrote:
> On February 7, 2014 9:58:14 PM CET, Thom Brown wrote:
>>On 7 February 2014 19:35, Andres Freund wrote:
>>> 0004: wal_decoding: Documentation for replication slots and changeset
>>extraction
>>
>>The usage of pg_create_decoding_replication_slot does show the "(1
>>row)" line.
>>
>>The output of "SELECT * FROM pg_replication_slots;" is out-of-date.
>>
>>There appears to be a column named "slot_name" and "slottype". Could
>>one of these have or not have the underscore for consistency?
>>
>>The example also shows output from pg_decoding_slot_get_changes after
>>inserting 2 rows, but when I run the same example, there are no rows
>>returned:
>>
>># BEGIN;
>>BEGIN
>>
>>*# INSERT INTO data(data) VALUES('1');
>>INSERT 0 1
>>
>>*# INSERT INTO data(data) VALUES('1');
>>INSERT 0 1
>>
>>*# COMMIT;
>>COMMIT
>>
>># SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
>>'include-xids', '0');
>> location | xid | data
>>--+-+--
>>(0 rows)
>>
>>
>>I inserted a single row outside of a transaction, and got the expected
>>output. Then I ran the above again, and got an output, but an
>>unexpected one:
>>
>>SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
>>'include-xids', '0');
>> location | xid | data
>>---+-+---
>> 0/16C8B90 | 769 | BEGIN
>> 0/16C8D50 | 769 | table "data": INSERT: id[int4]:3 data[text]:1
>> 0/16C8D50 | 769 | COMMIT
>>(3 rows)
>>
>>And running the transaction with inserts again, there's no output from
>>that same function command. I always get an output from isolated
>>INSERT statements. I should point out that in my .psqlrc file I have
>>"\set ON_ERROR_ROLLBACK". If I use psql -X, this symptom no longer
>>occurs, so I think the automatic savepoints are interfering, and the
>>effect appears to be inconsistent.
>
> More complete answer later, but any chance you're using synchronous commit =
> off?
No:
# show synchronous_commit ;
synchronous_commit
on
(1 row)
My custom config is:
wal_level = 'logical'
max_replication_slots = '1'
shared_buffers = 3900MB
temp_buffers = 16MB
work_mem = 16MB
maintenance_work_mem = 256MB
checkpoint_segments = 32
random_page_cost = 1.1
effective_cache_size = 12GB
logging_collector = on
log_line_prefix = '%t [%p]: [%l-1] user=%u,db=%d,client=%h '
--
Thom
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.5
On February 7, 2014 9:58:14 PM CET, Thom Brown wrote:
>On 7 February 2014 19:35, Andres Freund wrote:
>> 0004: wal_decoding: Documentation for replication slots and changeset
>extraction
>
>The usage of pg_create_decoding_replication_slot does show the "(1
>row)" line.
>
>The output of "SELECT * FROM pg_replication_slots;" is out-of-date.
>
>There appears to be a column named "slot_name" and "slottype". Could
>one of these have or not have the underscore for consistency?
>
>The example also shows output from pg_decoding_slot_get_changes after
>inserting 2 rows, but when I run the same example, there are no rows
>returned:
>
># BEGIN;
>BEGIN
>
>*# INSERT INTO data(data) VALUES('1');
>INSERT 0 1
>
>*# INSERT INTO data(data) VALUES('1');
>INSERT 0 1
>
>*# COMMIT;
>COMMIT
>
># SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
>'include-xids', '0');
> location | xid | data
>--+-+--
>(0 rows)
>
>
>I inserted a single row outside of a transaction, and got the expected
>output. Then I ran the above again, and got an output, but an
>unexpected one:
>
>SELECT * FROM pg_decoding_slot_get_changes('regression_slot', 'now',
>'include-xids', '0');
> location | xid | data
>---+-+---
> 0/16C8B90 | 769 | BEGIN
> 0/16C8D50 | 769 | table "data": INSERT: id[int4]:3 data[text]:1
> 0/16C8D50 | 769 | COMMIT
>(3 rows)
>
>And running the transaction with inserts again, there's no output from
>that same function command. I always get an output from isolated
>INSERT statements. I should point out that in my .psqlrc file I have
>"\set ON_ERROR_ROLLBACK". If I use psql -X, this symptom no longer
>occurs, so I think the automatic savepoints are interfering, and the
>effect appears to be inconsistent.
More complete answer later, but any chance you're using synchronous commit =
off?
Thanks for looking,
Andres
--
Please excuse brevity and formatting - I am writing this on my mobile phone.
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
Re: [HACKERS] Changeset Extraction v7.5
On 7 February 2014 20:58, Thom Brown wrote: > On 7 February 2014 19:35, Andres Freund wrote: >> 0004: wal_decoding: Documentation for replication slots and changeset >> extraction > > The usage of pg_create_decoding_replication_slot does show the "(1 row)" line. I mean "doesn't show" of course. :) -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
