Re: Different compression methods for FPI

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, pgsql-hackers(at)postgresql(dot)org, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Different compression methods for FPI
Date: 2022-09-06 06:47:05
Message-ID: Yxbs6TZAh5KwMg/v@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Sep 04, 2022 at 07:23:20PM -0500, Justin Pryzby wrote:
> That's also hitting an elog().
>
> 2022-09-04 15:56:29.916 CDT startup[2625] FATAL: XX000: failed to restore block image
> 2022-09-04 15:56:29.916 CDT startup[2625] DETAIL: image at 0/1D11CB8 compressed with zstd not supported by build, block 0
> 2022-09-04 15:56:29.916 CDT startup[2625] CONTEXT: WAL redo at 0/1D11CB8 for Heap/DELETE: off 50 flags 0x00 KEYS_UPDATED ; blkref #0: rel 1663/16384/2610, blk 4 FPW
> 2022-09-04 15:56:29.916 CDT startup[2625] LOCATION: XLogReadBufferForRedoExtended, xlogutils.c:396
>
> I guess it should be promoted to an ereport(), since it's now a
> user-facing error rathere than an internal one.
>
> diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
> index 0cda22597fe..01c7454bcc7 100644
> --- a/src/backend/access/transam/xlogutils.c
> +++ b/src/backend/access/transam/xlogutils.c
> @@ -393,7 +393,11 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
> prefetch_buffer);
> page = BufferGetPage(*buf);
> if (!RestoreBlockImage(record, block_id, page))
> - elog(ERROR, "failed to restore block image");
> + ereport(ERROR,
> + errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("failed to restore block image"),
> + errdetail("%s", record->errormsg_buf));
> +

Yes, you are right here. elog()'s should never be used for things
that could be triggered by the user, even if this one depends on the
build options. I think that the error message ought to be updated as
"could not restore block image" instead, to be more in line with the
project policy.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Antonin Houska 2022-09-06 07:26:08 Return value of PathNameOpenFile()
Previous Message Michael Paquier 2022-09-06 06:41:41 Re: [PATCH] Tab completion for SET COMPRESSION