Hi,
On Tue, Jul 29, 2025 at 3:56 PM Stepan Neretin <slpmcf@gmail.com> wrote:
>
> We’ve been thinking about how to make WAL archiving faster.
> This topic was previously discussed in [1], and we’ve taken a first step by implementing the attached patch, which
addssupport for archiving multiple WAL files in one go.
> The idea is straightforward: instead of invoking the archive command or callback once per WAL file, we allow passing
abatch of files.
>
Thanks for the patch!
My first comments are related to code style and naming. I believe that
fixing them will make the review process more convenient.
1)
Please, try to make your diff with vanilla code as small as possible.
If you believe that some formatting of the code "around" your solution
can be made better, you can provide a different .patch file, containing
such corrections.
2)
You have added "archive_files_cb". Maybe it should be renamed to
something like "archive_multiple_files_cb"? During review, I regularly
messed up these callbacks and functions with the same
naming (...file and ...files).
3)
shell_archive.c : I think that we should place "shell_archive_file" code
over "run_archive_command" code. Again, it will noticeably reduce diff.
4)
pgarch.c : definition of ArchiveXlogArg and ArchiveFilesArg structures
and ArchiveCallbackFn type definition should be placed in the head of the file.
5)
"pgarch_ArchiverCopyLoopMulti" function :
I guess that "ArchiveCallbacks->check_configured_cb != NULL" should be
placed at the beginning of the while loop (by analogy with
pgarch_ArchiverCopyLoop).
6)
Please, run pgindent on your patch.
--
Best regards,
Daniil Davydov
