On Thu, Apr 27, 2023 at 04:22:08PM +0200, Daniel Gustafsson wrote:
> I'm not sure I follow. While "." and ".." and hidden files share a leading '.'
> character they are vastly different and combining them in the same check would
> trade lines of code for readability. I'm not sure that's a net win. If you
> mean that the checks should be reordered such that hidden files are checked for
> before "pgsql_tmp", then that seems equally arbitrary no? There is no reason
> to believe that hidden files would be more frequently found than "pgsql_tmp
> files. I don't have strong opinions on the order, I'm just not sure either is
> better than the other.
FWIW, I don't have an issue with the order of the checks as presented
in v2.
> The commit message for 6ad8ac60262 doesn't explain why pgsql_tmp was left out
> of the excludeFiles table, but my guess is that it's either an optimization or
> a deliberate choice to not DEBUG1 log skipping temporary files. I didn't go
> digging in the archives to find the corresponding thread but there might be
> clues to be had there.
FWIW, here is the thread:
https://www.postgresql.org/message-id/CAB7nPqSNFm2Lz6jASj1RGvAdod1W8ZmHfvML3M7gDnBQ3x6QMw@mail.gmail.com
I think that you're right, the idea is to avoid the random noise
caused by these temp files and their names. This elog has been useful
for debugging in the past for the fixed entries, at least for me.
While on it, it strikes me that we should have a check on
PG_TEMP_FILES_DIR in basebackup.c's sendDir()? Okay, that's the same
as PG_TEMP_FILE_PREFIX, but pg_checksums and pg_rewind check for
*both* patterns so that feels inconsistent to me. This should not be
in excludeDirContents, because we don't want an empty folder in this
case.
--
Michael
