On Thu, Mar 4, 2021 at 8:03 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> Having said that, I think we use
> replication origins to test it. For example:
>
> Create Table t1(c1 int);
>
> SELECT pg_replication_origin_create('regress');
> SELECT pg_replication_origin_session_setup('regress');
> SELECT pg_replication_origin_xact_setup('0/aabbccdd', '2013-01-01 00:00');
> Begin;
> Select txid_current();
> Insert into t1 values(1);
> Prepare Transaction 'foo';
> SELECT pg_replication_origin_xact_setup('0/aabbccee', '2013-01-01 00:00');
> Rollback Prepared 'foo';
> SELECT pg_replication_origin_session_progress(true);
>
> -- Restart the server
> SELECT pg_replication_origin_session_setup('regress');
> SELECT pg_replication_origin_session_progress(true);
>
> The value before the patch will be aabbccdd and after the patch, it
> will be aabbccee.
>
> I think here we have a slight timing thing which is if the checkpoint
> happens before restart then the problem won't occur, not sure but
> maybe increase the checkpoint_timeout as well to make it reproducible.
> I am of opinion that this area won't change much and anyway once
> subscriber-side 2PC feature gets committed, we will have few tests
> covering this area but I am fine if you still insist to have something
> like above and think that we don't need to bother much about timing
> stuff.
>I have just checked via code coverage that we don't seem to have tests for recovery of replication origin advance for commit [1], see function xact_redo_commit. Similarly, a similar code is not covered for prepare [2], see EndPrepare. I think overall the test cases for replication origins are not very many. Now, I think if we want to have more tests in this area then we need to look at it more broadly. I think it could be that currently only subscriber-side covers some part of origins testing, otherwise, we don't have a detailed test suite by using replication origin APIs and second is probably it might be tricky to write a reliable recovery test case. One idea could be to just write a test for the non-recovery code path (the code added in function RecordTransactionAbortPrepared) and leave recovery testing for now. [1] - https://coverage.postgresql.org/src/backend/access/transam/xact.c.gcov.html [2] - https://coverage.postgresql.org/src/backend/access/transam/twophase.c.gcov.html -- With Regards, Amit Kapila.
