Re: pg_verifybackup: TAR format backup verification - Mailing list pgsql-hackers

On Thu, Aug 1, 2024 at 6:48 PM Amul Sul <sulamul@gmail.com> wrote:
>
> On Thu, Aug 1, 2024 at 1:37 AM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Wed, Jul 31, 2024 at 9:28 AM Amul Sul <sulamul@gmail.com> wrote:
> > > Fixed -- I did that because it was part of a separate group in pg_basebackup.
> >
[...]
> > Out of time for today, will look again soon. I think the first few of
> > these are probably pretty much ready for commit already, and with a
> > little more adjustment they'll probably be ready up through about
> > 0006.
> >
>
> Sure, thank you.
>

The v4 version isn't handling the progress report correctly because
the total_size calculation was done in verify_manifest_entry(), and
done_size was updated during the checksum verification. This worked
well for the plain backup but failed for the tar backup, where
checksum verification occurs right after verify_manifest_entry(),
leading to incorrect total_size in the progress report output.

Additionally, the patch missed the final progress_report(true) call
for persistent output, which is called from verify_backup_checksums()
for the plain backup but never for tar backup verification. To address
this, I moved the first and last progress_report() calls to the main
function. Although this is a small change, I placed it in a separate
patch, 0009, in the attached version.

In addition to these changes, the attached version includes
improvements in code comments, function names, and their arrangements
in astreamer_verify.c.

Please consider the attached version for the review.

Regards,
Amul