Thread

  1. Re: Making WAL archiving faster — multi-file support and async ideas

    Daniil Davydov <3danissimo@gmail.com> — 2025-08-04T05:59:40Z

    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 adds support 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 a batch 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