Horton

Re: pg_verifybackup: TAR format backup verification

Amul Sul <sulamul@gmail.com>

View thread View on postgr.es
From: Amul Sul <sulamul@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Sravan Kumar <sravanvcybage@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-08-02T11:43:02Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Adjust json_manifest_per_file_callback API in one more place.

  2. File size in a backup manifest should use uint64, not size_t.

  3. Fix some pg_verifybackup issues reported by Coverity.

  4. pg_verifybackup: Move some declarations to new pg_verifybackup.h

  5. pg_verifybackup: Move skip_checksums into verifier_context.

  6. Improve file header comments for astramer code.

  7. Move astreamer (except astreamer_inject) to fe_utils.

  8. Move recovery injector astreamer to a separate header file.

  9. Rename bbstreamer to astreamer.