Feedback-ID: ie0a040ee:Fastmail
Message-ID: <e191da9a-0966-841c-8842-14aa1128957b@eisentraut.org>
Date: Mon, 3 Jul 2023 09:47:05 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.12.0
Subject: Re: pg_upgrade --copy-file-range
Content-Language: en-US
To: Thomas Munro <thomas.munro@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <CA+hUKGKe7Hb0-UNih8VD5UNZy5-ojxFb3Pr3xSBBL8qj2M2=dQ@mail.gmail.com>
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <CA+hUKGKe7Hb0-UNih8VD5UNZy5-ojxFb3Pr3xSBBL8qj2M2=dQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/e191da9a-0966-841c-8842-14aa1128957b%40eisentraut.org>
Precedence: bulk

On 02.06.23 21:30, Thomas Munro wrote:
> I was just in a pg_upgrade unconference session at PGCon where the
> lack of $SUBJECT came up.  This system call gives the kernel the
> option to use fast block cloning on XFS, ZFS (as of very recently),
> etc, and works on Linux and FreeBSD.  It's probably much the same as
> --clone mode on COW file systems, except that is Linux-only.  On
> overwrite file systems (ie not copy-on-write, like ext4), it may also
> be able to push copies down to storage hardware/network file systems.
> 
> There was something like this in the nearby large files patch set, but
> in that version it just magically did it when available in --copy
> mode.  Now I think the user should have to have to opt in with
> --copy-file-range, and simply to error out if it fails.  It may not
> work in some cases -- for example, the man page says that older Linux
> systems can fail with EXDEV when you try to copy across file systems,
> while newer systems will do something less efficient but still
> sensible internally; also I saw a claim that some older versions had
> weird bugs.  Better to just expose the raw functionality and let users
> say when they want it and read the error if it fail, I think.

When we added --clone, copy_file_range() was available, but the problem 
was that it was hard for the user to predict whether you'd get the fast 
clone behavior or the slow copy behavior.  That's the kind of thing you 
want to know when planning and testing your upgrade.  At the time, there 
were patches passed around in Linux kernel circles that would have been 
able to enforce cloning via the flags argument of copy_file_range(), but 
that didn't make it to the mainline.

So, yes, being able to specify exactly which copy mechanism to use makes 
sense, so that users can choose the tradeoffs.

About your patch:

I think you should have a "check" function called from 
check_new_cluster().  That check function can then also handle the "not 
supported" case, and you don't need to handle that in 
parseCommandLine().  I suggest following the clone example for these, 
since the issues there are very similar.