Hi, Thanks for the warning fix, I will also look at the cassert case soon.
I have been adding more test cases to this patch. I added a TAP test which now allows us to do a concurrent ROLLBACK PREPARED when the walsender is in the midst of decoding this very prepared transaction. Have added a "decode-delay" parameter to test_decoding via which each apply call sleeps for a few configurable number of seconds allowing us to have deterministic rollback in parallel. This logic seems to work ok. However, I am battling an issue with invalidations now. Consider the below test case: CREATE TABLE test_prepared1(id integer primary key); -- test prepared xact containing ddl 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'; COMMIT PREPARED 'test_prepared#3'; SELECT data FROM pg_logical_slot_get_changes(..) <-- this shows the 2PC being decoded appropriately -- make sure stuff still works INSERT INTO test_prepared1 VALUES (8); SELECT data FROM pg_logical_slot_get_changes(..) The last pg_logical_slot_get_changes call, shows: table public.test_prepared1: INSERT: id[integer]:8 whereas since the 2PC committed, it should have shown: table public.test_prepared1: INSERT: id[integer]:8 data[text]:null This is an issue because of the way we are handling invalidations. We don't allow ReorderBufferAddInvalidations() at COMMIT PREPARE time since we assume that handling them at PREPARE time is enough. Apparently, it's not enough. Am trying to allow invalidations at COMMIT PREPARE time as well, but maybe calling ReorderBufferAddInvalidations() blindly again is not a good idea. Also, if I do that, then I am getting some restart_lsn inconsistencies which causes subsequent pg_logical_slot_get_changes() calls to re-decode older records. I continue to investigate. I am attaching the latest WIP patch. This contains the additional TAP test changes. Regards, Nikhils On 8 December 2017 at 01:15, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 12/7/17 08:31, Peter Eisentraut wrote: >> On 12/4/17 10:15, Nikhil Sontakke wrote: >>> PFA, latest patch for this functionality. >> >> This probably needs documentation updates for the logical decoding chapter. > > You need the attached patch to be able to compile without warnings. > > Also, the regression tests crash randomly for me at > > frame #4: 0x000000010a6febdb > postgres`heap_prune_record_prunable(prstate=0x00007ffee5578990, xid=0) > at pruneheap.c:625 > 622 * This should exactly match the PageSetPrunable macro. We > can't store > 623 * directly into the page header yet, so we update working > state. > 624 */ > -> 625 Assert(TransactionIdIsNormal(xid)); > 626 if (!TransactionIdIsValid(prstate->new_prune_xid) || > 627 TransactionIdPrecedes(xid, prstate->new_prune_xid)) > 628 prstate->new_prune_xid = xid; > > Did you build with --enable-cassert? > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services
2pc_logical_12_12_17_wip.patch
Description: Binary data
