Re: pg_combinebackup --copy-file-range

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Cc: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_combinebackup --copy-file-range
Date: 2024-03-29 14:23:16
Message-ID: CA+TgmoZ-DsjHPSznS73Pk258Qrc-vu4FnU1sMdXbJp256mpP=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 26, 2024 at 2:03 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Pyhalov 2024-03-29 14:46:31 Re: Partial aggregates pushdown
Previous Message Robert Treat 2024-03-29 14:16:32 Re: DOCS: add helpful partitioning links