logical changeset generation v4

Started by Andres Freundabout 13 years ago62 messageshackers
Jump to latest
#1Andres Freund
andres@anarazel.de

Hi everyone,

Here is the newest version of logical changeset generation.

Changes since last time round:
* loads and loads of bugfixes
* crash/restart persistency of in-memory structures in a crash safe manner
* very large transaction support (spooling to disk)
* rebased onto the newest version of xlogreader

Overview over the patches:

Xlogreader (separate patch):
[01]: Centralize Assert* macros into c.h so its common between backend/frontend
[02]: Provide a common malloc wrappers and palloc et al. emulation for frontend'ish environs
[03]: Split out xlog reading into its own module called xlogreader
[04]: Add pg_xlogdump contrib module

Those seem to be ready baring some infrastructure work around common
backend/frontend code for xlogdump.

Add capability to map from (tablespace, relfilenode) to pg_class.oid:
[05]: Add a new RELFILENODE syscache to fetch a pg_class entry via (reltablespace, relfilenode)
[06]: Add RelationMapFilenodeToOid function to relmapper.c
[07]: Add pg_relation_by_filenode to lookup up a relation by (tablespace, filenode)

Imo those are pretty solid although there are some doubts about the
correctness of [05]Add a new RELFILENODE syscache to fetch a pg_class entry via (reltablespace, relfilenode) which I think are all fixed in this version:

The fundamental problem of adding a (tablespace, relfilenode) syscache
is that no unique index exists in pg_class over (relfilenode,
reltablespace) because relfilenode is set to a '0' (aka InvalidOid) when
the table is either a shared table or a nailed table. This cannot really
be changed as pg_class.relfilenode is not authoritative for those and
can possibly not even accessed (different table, early startup). We also
don't want to rely on the null bitmap, so we can't set it to NULL.

The reason why I think it is safe to use the added RELFILENODE syscache
as I have in those patches is that when looking a (tablespace, filenode)
pair up none of those duplicat '0' values will get looked up as there is
no point in looking up an invalid relfilenode. Instead the shared/nailed
relfilenodes will have to get mapped via RelationMapFilenodeToOid.

The alternative here seems to be to invent an own attoptcache style but
given that the above syscache is fairly performance critical and should
do invalidations in a sensible manner that seems to be an unnecessary
amount of code.

Any opinions here?

[08]: wal_decoding: Introduce InvalidCommandId and declare that to be the new maximum for CommandCounterIncrement

Its useful to represent values that are not a valid CommandId. Add such
a representation.
Imo this is straightforward and easy.

[09]: Adjust all *Satisfies routines to take a HeapTuple instead of a HeapTupleHeader

For timetravel access to the catalog we need to be able to lookup (cmin,
cmax) pairs of catalog rows when were 'inside' that TX. This patch just
adapts the signature of the *Satisfies routines to expect a HeapTuple
instead of a HeapTupleHeader. The amount of changes for that is fairly
low as the HeapTupleSatisfiesVisibility macro already expected the
former.

It also makes sure the HeapTuple fields are setup in the few places that
didn't already do so.

[10]: wal_decoding: Allow walsender's to connect to a specific database

For logical decoding we need to be able access the schema of a database
- for that we need to be connected to a database. Thus allow recovery
connections to connect to a specific database.

This patch currently has the disadvantage that its not possible anymore
to connect to a database thats actually named "replication" as the
decision whether a connection goes to a database or not is made based
uppon dbname != replication.

Better ideas?

[11]: wal_decoding: Add alreadyLocked parameter to GetOldestXminNoLock

Pretty boring preparatory for being able to nail a certain xid as the
global horizon. I don't think there is much to be said about this
anymore, it already has been somewhat discussed.

[12]: wal_decodign: Log xl_running_xact's at a higher frequency than checkpoints are done

Make the bgwriter emit a xl_running_xacts record every 15s if there is
xlog activity in the system.
Imo this isn't too complicated and already beneficial for HS so it could
be applied separately.

[13]: copydir: make fsync_fname public

This should probably go to some other file, fd.[ch]? Otherwise its
pretty trivial.

[14]: wal decoding: Add information about a tables primary key to struct RelationData

Back when discussing the first prototype of BDR Heikki was concerned of
doing a search for the primary key during heap_delete. I agree that that
isn't really a good idea.
So remember the primary key (or a candidate key) when looking through
the available indexes in RelationGetIndexList().

I don't really like the name rd_primary as it also contains candidate
keys (i.e. indimmediate, inunique, noexpression, notnull), better
suggestions?

I don't think there is too much debatable in here, but there is no
independent benefit of applying it separately.

[15]: wal decoding: Introduce wal decoding via catalog timetravel

The heart of changeset generation.

Built out of several parts:

* snapshot building infrastructure
* transaction reassembly
* shared memory state for replication slots
* new wal_level=logical that catches more data
* (local) output plugin interface
* (external) walsender interface

[16]: wal decoding: Add a simple decoding module in contrib named 'test_decoding'

An example output plugin also used in regression tests

[17]: wal decoding: Introduce pg_receivellog, the pg_receivexlog equivalent for logical changes

An application to receive changes over the walsender/replication=1
interface.

[18]: wal_decoding: Add test_logical_replication extension for easier testing of logical decoding

An extension that allows to use logical decoding from sql. This isn't
really suitable for production, high performance use but its usefor for
development and more importantly it makes it relatively easy to write
regression tests without new infrastructure.

I am starting to be happy about the state of this!

Open issues & questions:
1) testing infrastructure
2) Determination of replication slots
3) Options for output plugins
4) the general walsender interface
5) Additional catalog tables

1) Currently all the tests are in patch [18]wal_decoding: Add test_logical_replication extension for easier testing of logical decoding which is a contrib
module. There are two reasons for putting them there:

First the tests need wal_level=logical which isn't the case with the
current regression tests.

Second, I don't think the test_logical_replication functions should live
in core as they shouldn't be used for a production replication scenario
(causes longrunning transactions, requires polling) , but I have failed
to find a neat way to include a contrib extension in the plain
regression tests.

2) Currently the logical replication infrastructure assigns a 'slot-id'
when a new replica is setup. That slot id isn't really nice
(e.g. "id-321578-3"). It also requires that [18]wal_decoding: Add test_logical_replication extension for easier testing of logical decoding keeps state in a global
variable to make writing regression tests easy.

I think it would be better to make the user specify those replication
slot ids, but I am not really sure about it.

3) Currently no options can be passed to an output plugin. I am thinking
about making "INIT_LOGICAL_REPLICATION 'plugin'" accept the now widely
used ('option' ['value'], ...) syntax and pass that to the output
plugin's initialization function.

4) Does anybody object to:
-- allocate a permanent replication slot
INIT_LOGICAL_REPLICATION 'plugin' 'slotname' (options);

-- stream data
START_LOGICAL_REPLICATION 'slotname' 'recptr';

-- deallocate a permanent replication slot
FREE_LOGICAL_REPLICATION 'slotname';

5) Currently its only allowed to access catalog tables, its fairly
trivial to extend this to additional tables if you can accept some
(noticeable but not too big) overhead for modifications on those tables.

I was thinking of making that an option for tables, that would be useful
for replication solutions configuration tables.

Further todo:
* don't reread so much WAL after a restart/crash
* better TOAST support, the current one can fail after A DROP TABLE
* only peg a new "catalog xmin" instead of the global xmin
* more docs about the internals
* nicer interface between snapbuild.c, reorderbuffer.c, decode.c and the
outside. There have been improvements vs 3.1 but not enough
* abort too old replication slots

Puh.

The current git tree is at:
git://git.postgresql.org/git/users/andresfreund/postgres.git branch xlog-decoding-rebasing-cf4
http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlog-decoding-rebasing-cf4

The xlogreader development happens xlogreader_4.

Input?

Greetings,

Andres Freund

PS: Thanks for the input & help so far!

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

Attachments:

0010-wal_decoding-Allow-walsender-s-to-connect-to-a-speci.patchtext/x-patch; charset=us-asciiDownload+41-15
0011-wal_decoding-Add-alreadyLocked-parameter-to-GetOldes.patchtext/x-patch; charset=us-asciiDownload+17-17
0012-wal_decodign-Log-xl_running_xact-s-at-a-higher-frequ.patchtext/x-patch; charset=us-asciiDownload+67-5
0001-Centralize-Assert-macros-into-c.h-so-its-common-betw.patchtext/x-patch; charset=us-asciiDownload+67-65
0002-Provide-a-common-malloc-wrappers-and-palloc-et-al.-e.patch.gzapplication/x-patch-gzipDownload
0003-Split-out-xlog-reading-into-its-own-module-called-xl.patch.gzapplication/x-patch-gzipDownload
0004-Add-pg_xlogdump-contrib-module.patchtext/x-patch; charset=us-asciiDownload+926-2
0005-wal_decoding-Add-a-new-RELFILENODE-syscache-to-fetch.patchtext/x-patch; charset=us-asciiDownload+14-1
0006-wal_decoding-Add-RelationMapFilenodeToOid-function-t.patchtext/x-patch; charset=us-asciiDownload+55-1
0007-wal-decoding-Add-pg_relation_by_filenode-to-lookup-u.patchtext/x-patch; charset=us-asciiDownload+104-2
0008-wal_decoding-Introduce-InvalidCommandId-and-declare-.patchtext/x-patch; charset=us-asciiDownload+3-3
0009-wal_decoding-Adjust-all-Satisfies-routines-to-take-a.patchtext/x-patch; charset=us-asciiDownload+83-35
0013-wal_decoding-copydir-make-fsync_fname-public.patchtext/x-patch; charset=us-asciiDownload+2-5
0014-wal-decoding-Add-information-about-a-tables-primary-.patchtext/x-patch; charset=us-asciiDownload+61-4
0015-wal-decoding-Introduce-wal-decoding-via-catalog-time.patch.gzapplication/x-patch-gzipDownload+1-1
0017-wal-decoding-Introduce-pg_receivellog-the-pg_receive.patchtext/x-patch; charset=us-asciiDownload+830-4
0018-wal_decoding-Add-test_logical_replication-extension-.patchtext/x-patch; charset=us-asciiDownload+796-1
0019-wal-decoding-design-document-v2.3-and-snapshot-build.patch.gzapplication/x-patch-gzipDownload
0016-wal-decoding-Add-a-simple-decoding-module-in-contrib.patchtext/x-patch; charset=us-asciiDownload+248-1
#2Josh Berkus
josh@agliodbs.com
In reply to: Andres Freund (#1)
Re: logical changeset generation v4

Andreas,

Is there a git fork for logical replication somewhere?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Andres Freund
andres@anarazel.de
In reply to: Josh Berkus (#2)
Re: logical changeset generation v4

Josh Berkus <josh@agliodbs.com> schrieb:

Andreas,

Is there a git fork for logical replication somewhere?

Check the bottom of the email ;)

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Abhijit Menon-Sen
ams@2ndQuadrant.com
In reply to: Josh Berkus (#2)
Re: logical changeset generation v4

At 2013-01-14 18:15:39 -0800, josh@agliodbs.com wrote:

Is there a git fork for logical replication somewhere?

git://git.postgresql.org/git/users/andresfreund/postgres.git, branch
xlog-decoding-rebasing-cf4 (and xlogreader_v4).

-- Abhijit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Abhijit Menon-Sen
ams@2ndQuadrant.com
In reply to: Andres Freund (#1)
Re: logical changeset generation v4

At 2013-01-15 02:38:45 +0100, andres@2ndquadrant.com wrote:

2) Currently the logical replication infrastructure assigns a
'slot-id' when a new replica is setup. That slot id isn't really
nice (e.g. "id-321578-3"). It also requires that [18] keeps state
in a global variable to make writing regression tests easy.

I think it would be better to make the user specify those replication
slot ids, but I am not really sure about it.

I agree, it would be better to let the user name the slot (and report an
error if the given name is already in use).

3) Currently no options can be passed to an output plugin. I am
thinking about making "INIT_LOGICAL_REPLICATION 'plugin'" accept the
now widely used ('option' ['value'], ...) syntax and pass that to the
output plugin's initialization function.

Sounds good.

4) Does anybody object to:
-- allocate a permanent replication slot
INIT_LOGICAL_REPLICATION 'plugin' 'slotname' (options);

-- stream data
START_LOGICAL_REPLICATION 'slotname' 'recptr';

-- deallocate a permanent replication slot
FREE_LOGICAL_REPLICATION 'slotname';

That looks fine, but I think it should be:

INIT_LOGICAL_REPLICATION 'slotname' 'plugin' (options);

i.e., swap 'plugin' and 'slotname' in your proposal to make the slotname
come first for all three commands. Not important, but a wee bit nicer.

-- Abhijit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#1)
Re: logical changeset generation v4

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

[08] wal_decoding: Introduce InvalidCommandId and declare that to be the new maximum for CommandCounterIncrement

This seems reasonable. Mainly it has the effect that a transaction can
have exactly one less command than before. I don't think this is a
problem for anyone in practice.

[09] Adjust all *Satisfies routines to take a HeapTuple instead of a HeapTupleHeader

Seemed okay when I looked at it.

Second, I don't think the test_logical_replication functions should live
in core as they shouldn't be used for a production replication scenario
(causes longrunning transactions, requires polling) , but I have failed
to find a neat way to include a contrib extension in the plain
regression tests.

I think this would work if you make a "stamp" file in the contrib
module, similar to how doc/src/sgml uses those.

--
Álvaro Herrera 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

#7Mark Kirkwood
mark.kirkwood@catalyst.net.nz
In reply to: Andres Freund (#1)
Re: logical changeset generation v4

On 15/01/13 14:38, Andres Freund wrote:

Hi everyone,

Here is the newest version of logical changeset generation.

I'm quite interested in this feature - so tried applying the 19 patches
to the latest 9.3 checkout. Patch and compile are good.

However portals seem busted:

bench=# BEGIN;
BEGIN
bench=# DECLARE c1 CURSOR FOR SELECT * FROM pgbench_accounts;
DECLARE CURSOR
bench=# FETCH 2 FROM c1;
aid | bid | abalance | filler

-----+-----+----------+---------------------------------------------------------
-----------------------------
1 | 1 | 0 |

2 | 1 | 0 |

(2 rows)

bench=# DELETE FROM pgbench_accounts WHERE CURRENT OF c1;
The connection to the server was lost. Attempting reset: Failed.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Mark Kirkwood
mark.kirkwood@catalyst.net.nz
In reply to: Mark Kirkwood (#7)
Re: logical changeset generation v4

On 15/01/13 17:37, Mark Kirkwood wrote:

On 15/01/13 14:38, Andres Freund wrote:

Hi everyone,

Here is the newest version of logical changeset generation.

I'm quite interested in this feature - so tried applying the 19
patches to the latest 9.3 checkout. Patch and compile are good.

However portals seem busted:

bench=# BEGIN;
BEGIN
bench=# DECLARE c1 CURSOR FOR SELECT * FROM pgbench_accounts;
DECLARE CURSOR
bench=# FETCH 2 FROM c1;
aid | bid | abalance | filler

-----+-----+----------+---------------------------------------------------------

-----------------------------
1 | 1 | 0 |

2 | 1 | 0 |

(2 rows)

bench=# DELETE FROM pgbench_accounts WHERE CURRENT OF c1;
The connection to the server was lost. Attempting reset: Failed.

Sorry - forgot to add: assert and debug build, and it is an assertion
failure that is being picked up:

TRAP: FailedAssertion("!(htup->t_tableOid != ((Oid) 0))", File:
"tqual.c", Line: 940)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Andres Freund
andres@anarazel.de
In reply to: Mark Kirkwood (#8)
Re: logical changeset generation v4

On 2013-01-15 17:41:50 +1300, Mark Kirkwood wrote:

On 15/01/13 17:37, Mark Kirkwood wrote:

On 15/01/13 14:38, Andres Freund wrote:

Hi everyone,

Here is the newest version of logical changeset generation.

I'm quite interested in this feature - so tried applying the 19 patches to
the latest 9.3 checkout. Patch and compile are good.

Thanks! Any input welcome.

The git tree might make it easier to follow development ;)

However portals seem busted:

bench=# BEGIN;
BEGIN
bench=# DECLARE c1 CURSOR FOR SELECT * FROM pgbench_accounts;
DECLARE CURSOR
bench=# FETCH 2 FROM c1;
aid | bid | abalance | filler

-----+-----+----------+---------------------------------------------------------

-----------------------------
1 | 1 | 0 |

2 | 1 | 0 |

(2 rows)

bench=# DELETE FROM pgbench_accounts WHERE CURRENT OF c1;
The connection to the server was lost. Attempting reset: Failed.

Sorry - forgot to add: assert and debug build, and it is an assertion
failure that is being picked up:

TRAP: FailedAssertion("!(htup->t_tableOid != ((Oid) 0))", File: "tqual.c",
Line: 940)

I unfortunately don't see the error here, I guess its related to how
stack is reused. But I think I found the error, check the attached patch
which I also pushed to the git repository.

Greetings,

Andres Freund

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

Attachments:

0001-wal_decoding-mergeme-Satisfies-Setup-a-correct-tup-t.patchtext/x-patch; charset=us-asciiDownload+2-1
#10Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#6)
Re: logical changeset generation v4

On 2013-01-15 01:00:00 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

I have some smaller bugfixes in my current version that you probably
don't have yet (on grounds of being fixed this weekend)... So we need to
be a bit careful not too loose those.

Second, I don't think the test_logical_replication functions should live
in core as they shouldn't be used for a production replication scenario
(causes longrunning transactions, requires polling) , but I have failed
to find a neat way to include a contrib extension in the plain
regression tests.

I think this would work if you make a "stamp" file in the contrib
module, similar to how doc/src/sgml uses those.

I tried that, the problem is not the building itself but getting it
installed into the temporary installation...
But anyway, testing decoding requires a different wal_level so I was
hesitant to continue on grounds of that alone.
Should we just up that? Its probably problematic for tests arround some
WAL optimizations an such?

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

#11Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#10)
Re: logical changeset generation v4

Andres Freund wrote:

On 2013-01-15 01:00:00 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

I have some smaller bugfixes in my current version that you probably
don't have yet (on grounds of being fixed this weekend)... So we need to
be a bit careful not too loose those.

Sure. Do you have them as individual commits? I'm assuming you rebased
the tree. Maybe in your reflog? IIRC I also have at least one minor
bug fix.

Second, I don't think the test_logical_replication functions should live
in core as they shouldn't be used for a production replication scenario
(causes longrunning transactions, requires polling) , but I have failed
to find a neat way to include a contrib extension in the plain
regression tests.

I think this would work if you make a "stamp" file in the contrib
module, similar to how doc/src/sgml uses those.

I tried that, the problem is not the building itself but getting it
installed into the temporary installation...

Oh, hm. Maybe the contrib module's make installcheck, then?

--
Álvaro Herrera 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

#12Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#11)
Re: logical changeset generation v4

On 2013-01-15 09:56:41 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

On 2013-01-15 01:00:00 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

I have some smaller bugfixes in my current version that you probably
don't have yet (on grounds of being fixed this weekend)... So we need to
be a bit careful not too loose those.

Sure. Do you have them as individual commits? I'm assuming you rebased
the tree. Maybe in your reflog? IIRC I also have at least one minor
bug fix.

I can check, which commit did you base your modifications on?

Second, I don't think the test_logical_replication functions should live
in core as they shouldn't be used for a production replication scenario
(causes longrunning transactions, requires polling) , but I have failed
to find a neat way to include a contrib extension in the plain
regression tests.

I think this would work if you make a "stamp" file in the contrib
module, similar to how doc/src/sgml uses those.

I tried that, the problem is not the building itself but getting it
installed into the temporary installation...

Oh, hm. Maybe the contrib module's make installcheck, then?

Thats what I do right now, but I really would prefer to have it checked
during normal make checks, installchecks aren't run all that commonly :(

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

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#12)
Re: logical changeset generation v4

Andres Freund <andres@2ndquadrant.com> writes:

On 2013-01-15 09:56:41 -0300, Alvaro Herrera wrote:

Oh, hm. Maybe the contrib module's make installcheck, then?

Thats what I do right now, but I really would prefer to have it checked
during normal make checks, installchecks aren't run all that commonly :(

Sure they are, in every buildfarm cycle. I don't see the problem.

(In the case of contrib, make installcheck is a whole lot faster than
make check, as well as being older. So I don't really see why you
think it's less used.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#13)
Re: logical changeset generation v4

On 2013-01-15 10:28:28 -0500, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

On 2013-01-15 09:56:41 -0300, Alvaro Herrera wrote:

Oh, hm. Maybe the contrib module's make installcheck, then?

Thats what I do right now, but I really would prefer to have it checked
during normal make checks, installchecks aren't run all that commonly :(

Sure they are, in every buildfarm cycle. I don't see the problem.

(In the case of contrib, make installcheck is a whole lot faster than
make check, as well as being older. So I don't really see why you
think it's less used.)

Oh. Because I was being dumb ;). And I admittedly never ran a buildfarm
animal so far.

But the other part of the problem is hiding in the unfortunately removed
part of the problem description - the tests require the non-default
options wal_level=logical and max_logical_slots=3+.
Is there a problem of making those the default in the buildfarm created
config?

I guess there would need to be an alternative output file for wal_level
< logical? Afaics there is no way to make a test conditional?

I shortly thought something like
DO $$
BEGIN
IF current_setting('wal_level') != 'df' THEN
RAISE FATAL 'wal_level needs to be logical';
END IF;
END
$$;
could be used to avoid creating a huge second output file, but we can't
raise FATAL errors from plpgsql.

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

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#14)
Re: logical changeset generation v4

Andres Freund <andres@2ndquadrant.com> writes:

But the other part of the problem is hiding in the unfortunately removed
part of the problem description - the tests require the non-default
options wal_level=logical and max_logical_slots=3+.

Oh. Well, that's not going to work.

Is there a problem of making those the default in the buildfarm created
config?

Even if we hacked the buildfarm script to do so, it'd be a nonstarter
because it would cause ordinary manual "make installcheck" to fail.

I think the only reasonable way to handle this would be to (1) make
"make installcheck" a no-op in this contrib module, and (2) make
"make check" work, being careful to start the test postmaster with
the necessary options.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#15)
Re: logical changeset generation v4

On 2013-01-15 11:10:22 -0500, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

But the other part of the problem is hiding in the unfortunately removed
part of the problem description - the tests require the non-default
options wal_level=logical and max_logical_slots=3+.

Oh. Well, that's not going to work.

An alternative would be to have max_logical_slots default to a low value
and make the amount of logged information a wal_level independent
GUC that can be changed on the fly.
ISTM that that would result in too complicated code to deal with other
backends not having the same notion of that value and such, but its
possible.

Is there a problem of making those the default in the buildfarm created
config?

Even if we hacked the buildfarm script to do so, it'd be a nonstarter
because it would cause ordinary manual "make installcheck" to fail.

I thought we could have a second expected file for that case. Not nice
:(

I think the only reasonable way to handle this would be to (1) make
"make installcheck" a no-op in this contrib module, and (2) make
"make check" work, being careful to start the test postmaster with
the necessary options.

Youre talking about adding a contrib-module specific make check or
changing the normal make check's wal_level?

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

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#16)
Re: logical changeset generation v4

Andres Freund <andres@2ndquadrant.com> writes:

On 2013-01-15 11:10:22 -0500, Tom Lane wrote:

I think the only reasonable way to handle this would be to (1) make
"make installcheck" a no-op in this contrib module, and (2) make
"make check" work, being careful to start the test postmaster with
the necessary options.

Youre talking about adding a contrib-module specific make check or
changing the normal make check's wal_level?

This contrib module's "make check" would change the wal_level. Global
change no good for any number of reasons, the most obvious being that
we want to be able to test other wal_levels too.

I'm not sure whether the "make check" infrastructure currently supports
passing arguments through to the test postmaster's command line, but it
shouldn't be terribly hard to add if not.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#12)
Re: logical changeset generation v4

Andres Freund wrote:

On 2013-01-15 09:56:41 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

On 2013-01-15 01:00:00 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

I have some smaller bugfixes in my current version that you probably
don't have yet (on grounds of being fixed this weekend)... So we need to
be a bit careful not too loose those.

Sure. Do you have them as individual commits? I'm assuming you rebased
the tree. Maybe in your reflog? IIRC I also have at least one minor
bug fix.

I can check, which commit did you base your modifications on?

Dunno. It's probably easier to reverse-apply the version you submitted
to see what changed, and then forward-apply again.

--
Álvaro Herrera 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

#19Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#18)
Re: logical changeset generation v4

On 2013-01-15 15:16:44 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

On 2013-01-15 09:56:41 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

On 2013-01-15 01:00:00 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

I've been giving a couple of these parts a look. In particular

[03] Split out xlog reading into its own module called xlogreader

Cleaned this one up a bit last week. I will polish it some more,
publish for some final comments, and commit.

I have some smaller bugfixes in my current version that you probably
don't have yet (on grounds of being fixed this weekend)... So we need to
be a bit careful not too loose those.

Sure. Do you have them as individual commits? I'm assuming you rebased
the tree. Maybe in your reflog? IIRC I also have at least one minor
bug fix.

I can check, which commit did you base your modifications on?

Dunno. It's probably easier to reverse-apply the version you submitted
to see what changed, and then forward-apply again.

There's at least the two attached patches...

Greetings,

Andres Freund

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

Attachments:

0001-xlogreader-fix.patchtext/x-patch; charset=us-asciiDownload+10-2
0001-xlogreader-use-correct-type.patchtext/x-patch; charset=us-asciiDownload+1-2
#20Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#1)
Re: logical changeset generation v4

Andres Freund wrote:

[09] Adjust all *Satisfies routines to take a HeapTuple instead of a HeapTupleHeader

For timetravel access to the catalog we need to be able to lookup (cmin,
cmax) pairs of catalog rows when were 'inside' that TX. This patch just
adapts the signature of the *Satisfies routines to expect a HeapTuple
instead of a HeapTupleHeader. The amount of changes for that is fairly
low as the HeapTupleSatisfiesVisibility macro already expected the
former.

It also makes sure the HeapTuple fields are setup in the few places that
didn't already do so.

I had a look at this part. Running the regression tests unveiled a case
where the tableOid wasn't being set (and thus caused an assertion to
fail), so I added that. I also noticed that the additions to
pruneheap.c are sometimes filling a tuple before it's strictly
necessary, leading to wasted work. Moved those too.

Looks good to me as attached.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

heaptuple-satisfies.patchtext/x-diff; charset=us-asciiDownload+119-68
#21Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Alvaro Herrera (#20)
#22Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#20)
#23Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#22)
#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#22)
#25Steve Singer
steve@ssinger.info
In reply to: Andres Freund (#1)
#26Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#23)
#27Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#26)
#28Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#25)
#29Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#25)
#30Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#29)
#31Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#30)
#32Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#1)
#33Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#32)
#34Joshua D. Drake
jd@commandprompt.com
In reply to: Robert Haas (#33)
#35Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#33)
#36Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Andres Freund (#32)
#37Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#33)
#38Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Andres Freund (#37)
#39Andres Freund
andres@anarazel.de
In reply to: Heikki Linnakangas (#36)
#40Andres Freund
andres@anarazel.de
In reply to: Heikki Linnakangas (#38)
#41Steve Singer
steve@ssinger.info
In reply to: Andres Freund (#39)
#42Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#37)
#43Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#42)
#44Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Robert Haas (#42)
#45Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#42)
#46Bruce Momjian
bruce@momjian.us
In reply to: Andres Freund (#45)
#47Andres Freund
andres@anarazel.de
In reply to: Heikki Linnakangas (#44)
#48Andres Freund
andres@anarazel.de
In reply to: Bruce Momjian (#46)
#49Bruce Momjian
bruce@momjian.us
In reply to: Andres Freund (#48)
#50Steve Singer
steve@ssinger.info
In reply to: Steve Singer (#41)
#51Steve Singer
steve@ssinger.info
In reply to: Andres Freund (#28)
#52Steve Singer
steve@ssinger.info
In reply to: Steve Singer (#41)
#53Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Andres Freund (#32)
#54Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#50)
#55Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#52)
#56Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#51)
#57Andres Freund
andres@anarazel.de
In reply to: Heikki Linnakangas (#53)
#58Steve Singer
steve@ssinger.info
In reply to: Andres Freund (#55)
#59Steve Singer
steve@ssinger.info
In reply to: Andres Freund (#56)
#60Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#56)
#61Andres Freund
andres@anarazel.de
In reply to: Steve Singer (#58)
#62Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#61)