On Fri, Jul 08, 2022 at 09:39:10PM +0530, Bharath Rupireddy wrote: > 0001 - there are many places where lstat/stat is being used - don't we > need to replace all or most of them with get_dirent_type?
It's been a while since I wrote this one, but I believe my intent was to replace as many [l]stat() calls in ReadDir()-style loops as possible with get_dirent_type(). Are there any that I've missed? > 0002 - I'm not quite happy with this patch, with the change, > checkpoint errors out, if it can't remove just a file - the comments > there says it all. Is there any strong reason for this change? Andres noted several concerns upthread. In short, ignoring unexpected errors makes them harder to debug and likely masks bugs. FWIW I agree that it is unfortunate that a relatively non-critical error here leads to checkpoint failures, which can cause much worse problems down the road. I think this is one of the reasons for moving tasks like this out of the checkpointer, as I'm trying to do with the proposed custodian process [0]. [0] https://commitfest.postgresql.org/38/3448/ -- Nathan Bossart Amazon Web Services: https://aws.amazon.com