Re: multi-install PostgresNode fails with older postgres versions

Andrew Dunstan <andrew@dunslane.net>

From: Andrew Dunstan <andrew@dunslane.net>
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Mark Dilger <mark.dilger@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-04-17T16:31:11Z
Lists: pgsql-hackers

Attachments

On 4/12/21 10:57 AM, Jehan-Guillaume de Rorthais wrote:
> On Mon, 12 Apr 2021 09:52:24 -0400
> Andrew Dunstan <andrew@dunslane.net> wrote:
>
>> On 4/12/21 8:59 AM, Jehan-Guillaume de Rorthais wrote:
>>> Hi,
>>>
>>> On Wed, 7 Apr 2021 20:07:41 +0200
>>> Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:
>>> [...]  
>>>>>> Let me know if it worth that I work on an official patch.      
>>>>> Let's give it a try ...    
>>>> OK  
>>> So, as promised, here is my take to port my previous work on PostgreSQL
>>> source tree.
>>>
>>> Make check pass with no errors. The new class probably deserve some own TAP
>>> tests.
>>>
>>> Note that I added a PostgresVersion class for easier and cleaner version
>>> comparisons. This could be an interesting take away no matter what.
>>>
>>> I still have some more ideas to cleanup, revamp and extend the base class,
>>> but I prefer to go incremental to keep things review-ability.
>>>  
>> Thanks for this. We have been working somewhat on parallel lines. With
>> your permission I'm going to take some of what you've done and
>> incorporate it in the patch I'm working on.
> The current context makes my weeks difficult to plan and quite chaotic, that's
> why it took me some days to produce the patch I promised.
>
> I'm fine with working with a common base code, thanks. Feel free to merge both
> code, we'll trade patches during review. However, I'm not sure what is the
> status of your patch, so I can not judge what would be the easier way to
> incorporate. Mine is tested down to 9.1 (9.0 was meaningless because of lack of
> pg_stat_replication) with:
>
> * get_new_node
> * init(allows_streaming => 1)
> * start
> * stop
> * backup
> * init_from_backup
> * wait_for_catchup
> * command_checks_all
>
> Note the various changes in my init() and new method allow_streaming(), etc.
>
> FYI (to avoid duplicate work), the next step on my todo was to produce some
> meaningful tap tests to prove the class.
>
>> A PostgresVersion class is a good idea - I was already contemplating
>> something of the kind.
> Thanks!
>


OK, here is more WIP on this item. I have drawn substantially on Mark's
and Jehan-Guillaime's work, but it doesn't really resemble either, and I
take full responsibility for it.

The guiding principles have been:

. avoid doing version tests, or capability tests which are the moral
equivalent, and rely instead on pure overloading.

. avoid overriding large pieces of code.


The last has involved breaking up some large subroutines (like init)
into pieces which can more sensibly be overridden. Breaking them up
isn't a bad thing to do anyway.

There is a new PostgresVersion object, but it's deliberately very
minimal. Since we're not doing version tests we don't need more complex
routines.


cheers


andrew




--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Commits

  1. Teach PostgresVersion all the ways to mark non-release code

  2. Make PostgresNode version aware

  3. Avoid unfortunate IPC::Run path caching in PostgresNode

  4. Change pg_ctl to detect server-ready by watching status in postmaster.pid.