Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Nathan Bossart <nathandbossart(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Date: 2022-08-09 03:41:14
Message-ID: CA+hUKGJt5AA5ioFG118F=+kW79Rnzicp=UmKG9ea8MqfsLnatQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 9, 2022 at 3:27 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Actually, having now read the patch, I don't think there is any
> part of 0002 that is a good idea. It's blithely removing the
> comments that explain why the existing coding is the way it is,
> and not providing a shred of justification for making checkpoints
> more brittle.

0002 also contradicts the original $SUBJECT and goal of this thread,
which is possibly why it was kept separate. I was only thinking of
committing 0001 myself, which is the one I'd reviewed an earlier
version of.

> I have not tried to analyze the error-handling properties of 0001,
> but if it's being equally cavalier then it shouldn't be committed
> either. Most of this behavior is the result of decades of hard-won
> experience; discarding it because it doesn't fit conveniently
> into some refactoring plan isn't smart.

0001 does introduce new errors, as mentioned in the commit message, in
the form of elevel ERROR passed into get_dirent_type(), which might be
thrown if your OS has no d_type and lstat() fails (also if you asked
to follow symlinks, but in those cases errors were already thrown).
But in those cases, it seems at least a little fishy that we ignored
errors from the existing lstat(). I wondered if that was because they
expected that any failure meant ENOENT and they wanted to tolerate
that, but that does not seem to be the case, so I considered the error
to be an improvement.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-08-09 03:50:18 Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Previous Message Nathan Bossart 2022-08-09 03:29:15 Re: out of date comment in commit_ts.c