On 2014-05-15 22:30:53 +0300, Heikki Linnakangas wrote:
> >Attached patch fixes things, but I want to add some regression tests
> >before commit.
>
> Looks good to me.
Attached are two patches. One for the unitialized dbId/tsId issue; one
for the decoding bug. The former should be backpatched.
Should you wonder about the slight reordering of the assignments in
RecordTransactionCommitPrepared() - I've made it more similar to
RecordTransactionCommit()
to make it easier to see they are equivalent.
Thanks for the testing!
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 07d0f4330f7c25bcec9e356527b0dc86372d2886 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 15 May 2014 22:23:12 +0200
Subject: [PATCH 1/2] Initialize all members of xl_xact_commit during prepared
transaction commits.
Commit dd428c79 added dbId and tsId to the xl_xact_commit struct but
missed that prepared transaction commits reuse that struct. Fix that.
Because those fields were used WAL logged unitialized a hot standby
node could miss relcache init file invalidations leading to errors like
ERROR: could not open file "...": No such file or directory
on the standby. A restart of the database is sufficient to fix the
problem.
As problems can only be triggered when a system table/index has been
rewritten in a transaction using two phase commit the problem is
unlikely to have affected many installations.
Found while investigating a logical decoding bugreport from Heikki.
Backpatch to 9.0 where the bug was introduced.
---
src/backend/access/transam/twophase.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 2cefa08..d5409a6 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2082,9 +2082,13 @@ RecordTransactionCommitPrepared(TransactionId xid,
/* Emit the XLOG commit record */
xlrec.xid = xid;
- xlrec.crec.xact_time = GetCurrentTimestamp();
+
xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
- xlrec.crec.nmsgs = 0;
+
+ xlrec.crec.dbId = MyDatabaseId;
+ xlrec.crec.tsId = MyDatabaseTableSpace;
+
+ xlrec.crec.xact_time = GetCurrentTimestamp();
xlrec.crec.nrels = nrels;
xlrec.crec.nsubxacts = nchildren;
xlrec.crec.nmsgs = ninvalmsgs;
--
2.0.0.rc2.4.g1dc51c6.dirty
>From de22d8e5308b1c4c509d8566317d9aea0956c2bc Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 15 May 2014 22:42:27 +0200
Subject: [PATCH 2/2] Fix typo-induced bug in decoding of prepared
transactions.
The decoding of prepared transaction commits accidentally used the xid
of the transaction performing the COMMIT PREPARED. Before
bb38fb0d43c8d that lead to those transactions not being decoded,
afterwards to a assertion failure.
Add tests for the decoding of prepared transactions.
Bug found and fixed by Heikki Linnakangas; new regression test by
Andres Freund.
---
contrib/test_decoding/Makefile | 2 +-
contrib/test_decoding/expected/ddl.out | 6 +--
contrib/test_decoding/expected/prepared.out | 82 +++++++++++++++++++++++++++++
contrib/test_decoding/sql/ddl.sql | 2 +-
contrib/test_decoding/sql/prepared.sql | 50 ++++++++++++++++++
src/backend/replication/logical/decode.c | 2 +-
6 files changed, 138 insertions(+), 6 deletions(-)
create mode 100644 contrib/test_decoding/expected/prepared.out
create mode 100644 contrib/test_decoding/sql/prepared.sql
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index 685986c..58e0f38 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -37,7 +37,7 @@ submake-isolation:
submake-test_decoding:
$(MAKE) -C $(top_builddir)/contrib/test_decoding
-REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary
+REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact binary prepared
regresscheck: all | submake-regress submake-test_decoding
$(MKDIR_P) regression_output
diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out
index 05a4bd3..37ff8b7 100644
--- a/contrib/test_decoding/expected/ddl.out
+++ b/contrib/test_decoding/expected/ddl.out
@@ -640,8 +640,8 @@ SELECT pg_drop_replication_slot('regression_slot');
(1 row)
/* check that we aren't visible anymore now */
-SELECT * FROM pg_stat_replication;
- pid | usesysid | usename | application_name | client_addr | client_hostname | client_port | backend_start | backend_xmin | state | sent_location | write_location | flush_location | replay_location | sync_priority | sync_state
------+----------+---------+------------------+-------------+-----------------+-------------+---------------+--------------+-------+---------------+----------------+----------------+-----------------+---------------+------------
+SELECT * FROM pg_replication_slots;
+ slot_name | plugin | slot_type | datoid | database | active | xmin | catalog_xmin | restart_lsn
+-----------+--------+-----------+--------+----------+--------+------+--------------+-------------
(0 rows)
diff --git a/contrib/test_decoding/expected/prepared.out b/contrib/test_decoding/expected/prepared.out
new file mode 100644
index 0000000..0f54f53
--- /dev/null
+++ b/contrib/test_decoding/expected/prepared.out
@@ -0,0 +1,82 @@
+-- predictability
+SET synchronous_commit = on;
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
+ ?column?
+----------
+ init
+(1 row)
+
+CREATE TABLE test_prepared1(id int);
+CREATE TABLE test_prepared2(id int);
+-- test simple successful use of a prepared xact
+BEGIN;
+INSERT INTO test_prepared1 VALUES (1);
+PREPARE TRANSACTION 'test_prepared#1';
+COMMIT PREPARED 'test_prepared#1';
+INSERT INTO test_prepared1 VALUES (2);
+-- test abort of a prepared xact
+BEGIN;
+INSERT INTO test_prepared1 VALUES (3);
+PREPARE TRANSACTION 'test_prepared#2';
+ROLLBACK PREPARED 'test_prepared#2';
+INSERT INTO test_prepared1 VALUES (4);
+-- test prepared xact ddl inside
+BEGIN;
+INSERT INTO test_prepared1 VALUES (5);
+ALTER TABLE test_prepared1 ADD COLUMN data text;
+INSERT INTO test_prepared1 VALUES (6, 'frakbar');
+PREPARE TRANSACTION 'test_prepared#3';
+-- test that we decode correctly while a uncommitted prepared xact
+-- with ddl exist.
+-- separate table because of the lock from the ALTER
+-- this will come before the '5' row above as it commits before it.
+INSERT INTO test_prepared2 VALUES (7);
+COMMIT PREPARED 'test_prepared#3';
+-- make sure stuff still works
+INSERT INTO test_prepared1 VALUES (8);
+INSERT INTO test_prepared2 VALUES (9);
+-- cleanup
+DROP TABLE test_prepared1;
+DROP TABLE test_prepared2;
+-- show results
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0');
+ data
+-------------------------------------------------------------------------
+ BEGIN
+ COMMIT
+ BEGIN
+ COMMIT
+ BEGIN
+ table public.test_prepared1: INSERT: id[integer]:1
+ COMMIT
+ BEGIN
+ table public.test_prepared1: INSERT: id[integer]:2
+ COMMIT
+ BEGIN
+ table public.test_prepared1: INSERT: id[integer]:4
+ COMMIT
+ BEGIN
+ table public.test_prepared2: INSERT: id[integer]:7
+ COMMIT
+ BEGIN
+ table public.test_prepared1: INSERT: id[integer]:5
+ table public.test_prepared1: INSERT: id[integer]:6 data[text]:'frakbar'
+ COMMIT
+ BEGIN
+ table public.test_prepared1: INSERT: id[integer]:8 data[text]:null
+ COMMIT
+ BEGIN
+ table public.test_prepared2: INSERT: id[integer]:9
+ COMMIT
+ BEGIN
+ COMMIT
+ BEGIN
+ COMMIT
+(30 rows)
+
+SELECT pg_drop_replication_slot('regression_slot');
+ pg_drop_replication_slot
+--------------------------
+
+(1 row)
+
diff --git a/contrib/test_decoding/sql/ddl.sql b/contrib/test_decoding/sql/ddl.sql
index 555a59c..3811786 100644
--- a/contrib/test_decoding/sql/ddl.sql
+++ b/contrib/test_decoding/sql/ddl.sql
@@ -334,4 +334,4 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
SELECT pg_drop_replication_slot('regression_slot');
/* check that we aren't visible anymore now */
-SELECT * FROM pg_stat_replication;
+SELECT * FROM pg_replication_slots;
diff --git a/contrib/test_decoding/sql/prepared.sql b/contrib/test_decoding/sql/prepared.sql
new file mode 100644
index 0000000..86fd030
--- /dev/null
+++ b/contrib/test_decoding/sql/prepared.sql
@@ -0,0 +1,50 @@
+-- predictability
+SET synchronous_commit = on;
+SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
+
+CREATE TABLE test_prepared1(id int);
+CREATE TABLE test_prepared2(id int);
+
+-- test simple successful use of a prepared xact
+BEGIN;
+INSERT INTO test_prepared1 VALUES (1);
+PREPARE TRANSACTION 'test_prepared#1';
+COMMIT PREPARED 'test_prepared#1';
+INSERT INTO test_prepared1 VALUES (2);
+
+-- test abort of a prepared xact
+BEGIN;
+INSERT INTO test_prepared1 VALUES (3);
+PREPARE TRANSACTION 'test_prepared#2';
+ROLLBACK PREPARED 'test_prepared#2';
+
+INSERT INTO test_prepared1 VALUES (4);
+
+-- test prepared xact ddl inside
+BEGIN;
+INSERT INTO test_prepared1 VALUES (5);
+ALTER TABLE test_prepared1 ADD COLUMN data text;
+INSERT INTO test_prepared1 VALUES (6, 'frakbar');
+PREPARE TRANSACTION 'test_prepared#3';
+
+-- test that we decode correctly while a uncommitted prepared xact
+-- with ddl exist.
+
+-- separate table because of the lock from the ALTER
+-- this will come before the '5' row above as it commits before it.
+INSERT INTO test_prepared2 VALUES (7);
+
+COMMIT PREPARED 'test_prepared#3';
+
+-- make sure stuff still works
+INSERT INTO test_prepared1 VALUES (8);
+INSERT INTO test_prepared2 VALUES (9);
+
+-- cleanup
+DROP TABLE test_prepared1;
+DROP TABLE test_prepared2;
+
+-- show results
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0');
+
+SELECT pg_drop_replication_slot('regression_slot');
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index 7b6114a..cc73652 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -225,7 +225,7 @@ DecodeXactOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
subxacts = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
invals = (SharedInvalidationMessage *) &(subxacts[xlrec->nsubxacts]);
- DecodeCommit(ctx, buf, r->xl_xid, xlrec->dbId,
+ DecodeCommit(ctx, buf, prec->xid, xlrec->dbId,
xlrec->xact_time,
xlrec->nsubxacts, subxacts,
xlrec->nmsgs, invals);
--
2.0.0.rc2.4.g1dc51c6.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
