public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Tomas Vondra <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re: pg_combinebackup --clone doesn't work
Date: Tue, 25 Jun 2024 15:21:37 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On 21.06.24 18:10, Tomas Vondra wrote:
> On 6/21/24 00:07, Tomas Vondra wrote:
>> Here's a fix adding the missing headers to pg_combinebackup, and fixing
>> some compile-time issues in the ifdef-ed block.
>>
>> I've done some basic manual testing today - I plan to test this a bit
>> more tomorrow, and I'll also look at integrating this into the existing
>> tests.
>>
> 
> Here's a bit more complete / cleaned patch, adding the testing changes
> in separate parts.
> 
> 0001 adds the missing headers / fixes the now-accessible code a bit
> 
> 0002 adds the --copy option for consistency with pg_upgrade

This looks good.

> 0003 adds the PG_TEST_PG_COMBINEBACKUP_MODE, so that we can override the
> copy method for tests

I had imagined that we combine PG_TEST_PG_UPGRADE_MODE and this new one 
into one setting.  But maybe that's something to consider with less time 
pressure for PG18.

 > I believe 0001-0003 are likely non-controversial, although if someone
 > could take a look at the Perl in 0003 that'd be nice. Also, 0002 seems
 > nice not only because of consistency with pg_upgrade, but it also makes
 > 0003 easier as we don't need to special-case the default mode etc.

Right, that was one of the reasons.

> 0004 tweaks two of the Cirrus CI tasks to use --clone/--copy-file-range

> I'm not sure about 0004 - I initially did this mostly to check we have
> the right headers on other platforms, but not sure we want to actually
> do this. Or maybe we want to test a different combination (e.g. also
> test the --clone on Linux)?

It's tricky to find the right balance here.  We had to figure this out 
for pg_upgrade as well.  I think your solution is good, and we should 
also add test coverage for pg_upgrade --copy-file-range in the same 
place, I think.

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pg_combinebackup --clone doesn't work
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox