Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Nathan Bossart <nathandbossart@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
Andres Freund <andres@anarazel.de>, "Bossart, Nathan" <bossartn@amazon.com>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-08-09T03:41:14Z
Lists: pgsql-hackers
On Tue, Aug 9, 2022 at 3:27 PM Tom Lane <tgl@sss.pgh.pa.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.
Commits
-
Expand the use of get_dirent_type(), shaving a few calls to stat()/lstat()
- bfb9dfd93720 16.0 landed
-
fsync pg_logical/mappings in CheckPointLogicalRewriteHeap().
- fd48e5f5d3a1 13.6 landed
- f862cc09fad1 10.20 landed
- 2c15b29f7c22 11.15 landed
- 2b7dbe4bd587 14.2 landed
- 1c6d055ba77a 12.10 landed
- 1fabec7d7c38 15.0 landed
-
Introduce logical decoding.
- b89e151054a0 9.4.0 cited