Re: Track replica origin progress for Rollback Prepared

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Nikhil Sontakke <nikhils(at)2ndquadrant(dot)com>
Subject: Re: Track replica origin progress for Rollback Prepared
Date: 2021-03-04 14:33:25
Message-ID: CAA4eK1LyFYQauvM=1ipbr16B8ob28d28Hf5Z+yWY0ow9utnMwA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 4, 2021 at 1:40 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Mar 03, 2021 at 09:06:11AM +0530, Amit Kapila wrote:
> That's perhaps not enough to be an actual objection, but I am not
> really comfortable with the concept of pushing into the tree code that
> would remain untested until all the remaining pieces get done, as that
> means that we have a point in the code history where some code is
> around, but sits around as idle, no? If this feature is not completed
> within the dev cycle, it would mean that some code remains around
> without any actual way to know if it is useful or not, released as
> such.
>

You are right but not sure if we should try to write the test case to
cover each and every line especially if the test case is sort of
timing dependent which I think is the case here. We don't have any
test even for the original feature committed at 1eb6d6527a, maybe, we
should write a one for that as well. 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.

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message walker 2021-03-04 14:34:38 011_crash_recovery.pl failes using wal_block_size=16K
Previous Message Georgios Kokolatos 2021-03-04 14:28:46 Re: [PATCH] postgres-fdw: column option to override foreign types