public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: checksum verification code breaks backups in v16-
Date: Tue, 3 Dec 2024 16:26:36 -0600
Message-ID: <Z0-FnG0zQ9xlciQP@nathan> (raw)
In-Reply-To: <CA+TgmoaWmO=V55Cy1brfww9ewjRrZE80ej8ox1iu2p2JhKKu_A@mail.gmail.com>
References: <CA+TgmoaWmO=V55Cy1brfww9ewjRrZE80ej8ox1iu2p2JhKKu_A@mail.gmail.com>
On Tue, Dec 03, 2024 at 11:39:43AM -0500, Robert Haas wrote:
> If we want a narrower and thus less-risky fix, we could consider just
> adjusting this code here:
>
> segmentno = atoi(segmentpath + 1);
> if (segmentno == 0)
> ereport(ERROR,
> (errmsg("invalid segment number %d in file \"%s\"",
> segmentno, filename)));
I'm only seeing this code in pg_checksums. Is that what you are proposing
to change?
> I think we could just replace the ereport() with verify_checksum =
> false (and update the comments). That would leave open the possibility
> of trying to checksum some random file with a name like
> this_file_should_not_be_here, which we'll interpret as segment 0
> because the name does not contain a dot; but if the file were called
> this.file.should.not.be.here, we'd not try to checksum it at all
> because "here" is not an integer. That's a little random, but it
> avoids taking a position on whether
> 025584a168a4b3002e19350bb8db0ebf1fd10235 is fully correct. It simply
> takes the narrower position that we shouldn't fail the entire backup
> because we're unable to perform a checksum calculation that in the
> worst case would only produce a warning anyway.
Hm.
> At the moment, I think I slightly prefer the narrower fix, but it
> might be a little too narrow, since the resultant behavior as
> described in the preceding paragraph is somewhat wonky and
> nonsensical.
Yeah, I'm a little worried that it is too narrow. I was hoping that the
refactoring commit was older, because then I think we'd be much more
willing to back-patch it. But there are a couple of v17 releases out there
already, so maybe it's not too much of a stretch. I guess I'm leaning
towards the back-patching idea...
--
nathan
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: checksum verification code breaks backups in v16-
In-Reply-To: <Z0-FnG0zQ9xlciQP@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox