Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "Bossart,
Nathan" <bossartn@amazon.com>, Tom Lane <tgl@sss.pgh.pa.us>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-02-03T04:15:08Z
Lists: pgsql-hackers
Attachments
- v7-0001-Replace-ReadDir-with-ReadDirExtended.patch (application/octet-stream) patch v7-0001
On Thu, Feb 3, 2022 at 12:07 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Wed, Feb 02, 2022 at 05:19:26PM +0530, Bharath Rupireddy wrote: > > On Wed, Feb 2, 2022 at 5:25 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > >> However, I'm not sure about the change to ReadDirExtended(). That might be > >> okay for CheckPointSnapBuild(), which is just trying to remove old files, > >> but CheckPointLogicalRewriteHeap() is responsible for ensuring that files > >> are flushed to disk for the checkpoint. If we stop reading the directory > >> after an error and let the checkpoint continue, isn't it possible that some > >> mappings files won't be persisted to disk? > > > > Unless I mis-read your above statement, with LOG level in > > ReadDirExtended, I don't think we stop reading the files in > > CheckPointLogicalRewriteHeap. Am I missing something here? > > ReadDirExtended() has the following comment: > > * If elevel < ERROR, returns NULL after any error. With the normal coding > * pattern, this will result in falling out of the loop immediately as > * though the directory contained no (more) entries. > > If there is a problem reading the directory, we will LOG and then exit the > loop. If we didn't scan through all the entries in the directory, there is > a chance that we didn't fsync() all the files that need it. Thanks. I get it. For syncing map files, we don't want to tolerate any errors, whereas removal of the old map files (lesser than cutoff LSN) can be tolerated in CheckPointLogicalRewriteHeap. Here's the v7 version using ReadDir for CheckPointLogicalRewriteHeap. Regards, Bharath Rupireddy.
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