Re: pg_verifybackup: TAR format backup verification

From: Amul Sul <sulamul(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Sravan Kumar <sravanvcybage(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: pg_verifybackup: TAR format backup verification
Date: 2024-08-02 11:43:02
Message-ID: CAAJ_b97TruRwr=FhQ_J62-adFi=ZUFT407mHFKyHmvUJy5tg+A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 1, 2024 at 6:48 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> On Thu, Aug 1, 2024 at 1:37 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > On Wed, Jul 31, 2024 at 9:28 AM Amul Sul <sulamul(at)gmail(dot)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

Attachment Content-Type Size
v5-0010-pg_verifybackup-Add-backup-format-and-compression.patch application/x-patch 8.7 KB
v5-0011-pg_verifybackup-Read-tar-files-and-verify-its-con.patch application/x-patch 24.2 KB
v5-0012-pg_verifybackup-Tests-and-document.patch application/x-patch 13.7 KB
v5-0009-Refactor-move-first-and-last-progress_report-call.patch application/x-patch 1.5 KB
v5-0008-Refactor-split-verify_control_file.patch application/x-patch 5.4 KB
v5-0007-Refactor-split-verify_file_checksum-function.patch application/x-patch 2.4 KB
v5-0005-Refactor-move-some-part-of-pg_verifybackup.c-to-p.patch application/x-patch 8.5 KB
v5-0004-Refactor-move-few-global-variable-to-verifier_con.patch application/x-patch 5.1 KB
v5-0006-Refactor-split-verify_backup_file-function.patch application/x-patch 5.6 KB
v5-0003-Refactor-move-astreamer-files-to-fe_utils-to-make.patch application/x-patch 7.1 KB
v5-0002-Refactor-Add-astreamer_inject.h-and-move-related-.patch application/x-patch 3.0 KB
v5-0001-Refactor-Rename-all-bbstreamer-references-to-astr.patch application/x-patch 106.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Rajesh Kokkonda 2024-08-02 11:45:39 Re: Memory growth observed with C++ application consuming libpq.dll on Windows
Previous Message Yasir 2024-08-02 11:36:35 Re: Memory growth observed with C++ application consuming libpq.dll on Windows