Message-ID: <537515ED.5060106@vmware.com>
Date: Thu, 15 May 2014 22:30:53 +0300
From: Heikki Linnakangas <hlinnakangas@vmware.com>
Organization: VMware
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:24.0) Gecko/20100101 Icedove/24.4.0
MIME-Version: 1.0
To: Andres Freund <andres@2ndquadrant.com>
CC: PostgreSQL-development <pgsql-hackers@postgreSQL.org>
Subject: Re: Logical replication woes
References: <5374F1FF.9000902@vmware.com> <5374F44B.1060003@vmware.com>
 <20140515174657.GI23662@alap3.anarazel.de>
In-Reply-To: <20140515174657.GI23662@alap3.anarazel.de>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 05/15/2014 08:46 PM, Andres Freund wrote:
> On 2014-05-15 20:07:23 +0300, Heikki Linnakangas wrote:
>> Ok, so the immediate cause was quick to find: when decoding a
>> commit-prepared WAL record, we have to use the XID from the record content
>> (patch attached). The XID in the record header is the XID of the transaction
>> doing the COMMIT PREPARED, which is always 0 after patch
>> bb38fb0d43c8d7ff54072bfd8bd63154e536b384 which causes the assertion. But it
>> was always wrong. After fixing, it no longer asserts or gives the above
>> "could not map filenode" error.
>>
>> However, it still doesn't seem right. When I do the above as a regular
>> transaction, ie:
>>
>> begin; insert into foo values (6);  alter table foo alter column id type
>> text; commit;
>>
>> pg_recvlogical prints this:
>>
>> BEGIN 708
>> table public.foo: INSERT: id[text]:'6'
>> COMMIT 708
>>
>> But if I do it as a prepared transaction:
>>
>> begin; insert into foo values (7);  alter table foo alter column id type
>> text; prepare transaction 'foo'; commit prepared 'foo';
>
> How very wierd. The reason for this is that
> RecordTransactionCommitPrepared() forgets to fill a couple of fields in
> xl_xact_commit. Any reason dbId/tsId aren't filled? They aren't strictly
> needed because afaics they're only looked at for relcache invalidations
> which prepared xacts don't support, but still?

Seems like an oversight in commit dd428c79, which added the fields to 
xl_xact_commit. They are needed. A prepared xact can indeed cause 
relcache invalidations, and removal of the init file. For example:

  begin; cluster pg_opclass using pg_opclass_oid_index ; prepare 
transaction 'foo'; commit prepared 'foo';

> Attached patch fixes things, but I want to add some regression tests
> before commit.

Looks good to me.

- Heikki


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