Re: logical decoding of two-phase transactions

On Tue, Apr 4, 2017 at 7:06 PM, Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru> wrote:
>
>> On 4 Apr 2017, at 04:23, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>>
>> I reviewed this patch but when I tried to build contrib/test_decoding
>> I got the following error.
>>
>
> Thanks!
>
> Yes, seems that 18ce3a4a changed ProcessUtility_hook signature.
> Updated.
>
>> There are still some unnecessary code in v5 patch.
>

Thank you for updating the patch!

> Actually second diff isn’t intended to be part of the patch, I've just shared
> the way I ran regression test suite through the 2pc decoding changing
> all commits to prepare/commits where commits happens only after decoding
> of prepare is finished (more details in my previous message in this thread).

Understood. Sorry for the noise.

>
> That is just argument against Andres concern that prepared transaction
> is able to deadlock with decoding process — at least no such cases in
> regression tests.
>
> And that concern is main thing blocking this patch. Except explicit catalog
> locks in prepared tx nobody yet found such cases and it is hard to address
> or argue about.
>

Hmm, I also has not found such deadlock case yet.

Other than that issue current patch still could not pass 'make check'
test of contrib/test_decoding.

*** 154,167 ****
(4 rows)

:get_with2pc
! data
! -------------------------------------------------------------------------
! BEGIN
! table public.test_prepared1: INSERT: id[integer]:5
! table public.test_prepared1: INSERT: id[integer]:6 data[text]:'frakbar'
! PREPARE TRANSACTION 'test_prepared#3';
! COMMIT PREPARED 'test_prepared#3';
! (5 rows)

-- make sure stuff still works
INSERT INTO test_prepared1 VALUES (8);
--- 154,162 ----
(4 rows)

:get_with2pc
! data
! ------
! (0 rows)

-- make sure stuff still works
INSERT INTO test_prepared1 VALUES (8);

I guess that the this part is a unexpected result and should be fixed. Right?

-----

*** 215,222 ****
-- If we try to decode it now we'll deadlock
SET statement_timeout = '10s';
:get_with2pc_nofilter
! -- FIXME we expect a timeout here, but it actually works...
! ERROR: statement timed out

RESET statement_timeout;
-- we can decode past it by skipping xacts with catalog changes
--- 210,222 ----
-- If we try to decode it now we'll deadlock
SET statement_timeout = '10s';
:get_with2pc_nofilter
! data
! ----------------------------------------------------------------------------
! BEGIN
! table public.test_prepared1: INSERT: id[integer]:10 data[text]:'othercol'
! table public.test_prepared1: INSERT: id[integer]:11 data[text]:'othercol2'
! PREPARE TRANSACTION 'test_prepared_lock'
! (4 rows)

RESET statement_timeout;
-- we can decode past it by skipping xacts with catalog changes

Probably we can ignore this part for now.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center