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

Hi,

On 2022-02-10 21:30:45 +0530, Bharath Rupireddy wrote:
> From 4801ff2c3b1e7bc7076205b676d4e3bc4a4ed308 Mon Sep 17 00:00:00 2001
> From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
> Date: Thu, 10 Feb 2022 15:58:58 +0000
> Subject: [PATCH v8] Replace ReadDir with ReadDirExtended
>
> Replace ReadDir with ReadDirExtended (in CheckPointSnapBuild) and
> get rid of lstat entirely.

I think this might be based on a slight misunderstanding / bad phrasing on my
part. We can use get_dirent_type() to optimize away the lstat on most
platforms, ReadDirExtended itself doesn't do that automatically. I was trying
to reference removing lstat calls by using get_dirent_type() in more places...

> We still use ReadDir in CheckPointLogicalRewriteHeap
> because unable to read directory would result a NULL from
> ReadDirExtended and we may miss to fsync the remaining map files,
> so here let's error out with ReadDir.

Then why is this skipping the lstat?

> Also, convert "could not parse filename" and "could not remove file"
> errors to LOG messages in CheckPointLogicalRewriteHeap. This will
> enable checkpoint not to waste the amount of work that it had done.

I still doubt this is a good idea.

Greetings,

Andres Freund