Date: Wed, 2 Feb 2022 10:37:38 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@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>
Subject: Re: Avoid erroring out when unable to remove or parse logical
 rewrite files to save checkpoint work
Message-ID: <20220202183738.GA746893@nathanxps13>
References: <773619.1642632823@sss.pgh.pa.us>
 <21F86990-CC52-42B2-8A85-A3DA110F57F1@amazon.com>
 <20220120194618.hmfd4kxkng2cgryh@alap3.anarazel.de>
 <614DB439-2AB6-4EDC-8CE0-159C7FEB21AB@amazon.com>
 <867F2E29-2782-4869-970E-B984C6D35A8F@amazon.com>
 <20220121194956.br76car5qzqhji5s@alap3.anarazel.de>
 <20220127010109.GA352793@nathanxps13>
 <CALj2ACWgKS=ZUJ0xcXhbqieJXMQxncLMQRx6nwtVc9Fp5H58RA@mail.gmail.com>
 <20220201235538.GA740616@nathanxps13>
 <CALj2ACXzMf+8-KoREutAA40MrC4sMEY2P1aLFD_RYysC1M2CZQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CALj2ACXzMf+8-KoREutAA40MrC4sMEY2P1aLFD_RYysC1M2CZQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20220202183738.GA746893%40nathanxps13>
Precedence: bulk

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.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com