Re: pg_combinebackup --copy-file-range

On Tue, Mar 26, 2024 at 2:03 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
> [ new patches ]

Tomas, thanks for pointing me to this email; as you speculated, gmail
breaks threading if the subject line changes.

The documentation still needs work here:

- It refers to --link mode, which is not a thing.

- It should talk about the fact that in some cases block-by-block
copying will still be needed, possibly mentioning that it specifically
happens when the old backup manifest is not available or does not
contain checksums or does not contain checksums of the right type, or
maybe just being a bit vague.

In copy_file.c:

- You added an unnecessary blank line to the beginning of a comment block.

- You could keep the strategy_implementation variable here. I don't
think that's 100% necessary, but now that you've got the code
structured this way, there's no compelling reason to remove it.

- I don't know what the +/* XXX maybe this should do the check
internally, same as the other functions? */ comment is talking about.

- Maybe these functions should have header comments.

-- 
Robert Haas
EDB: http://www.enterprisedb.com