15.11.2020 08:00, Alexander Lakhin wrote:
> And it rises another question, what if pg_ls_dir_files() is called for a
> directory where hundreds or thousands of files are really inaccessible
> due to restrictions?
> I mean that using CreateFile() in the modified stat() implementation can
> be rather expensive for an arbitrary file (and worse yet, for many files).
> On the positive side, for now pg_ls_dir_files() is called only from
> pg_ls_logdir, pg_ls_waldir, pg_ls_tmpdir, pg_ls_archive_statusdir, where
> having a bunch of files that are inaccessible for the postgres user is
> not expected anyway.
I've missed the fact that pg_ls_dir_files() just fails on first error,
so the existing coding would not cause performance issues. But such
behavior is somewhat dubious, because having an inaccessible file in a
directory of interest would make any of those calls fail
> But probably getting directory contents with correct file sizes (>4GB)
> in pg_ls_dir_files() can be implemented without calling
> CreateFile()/stat() at all (as ProcMon shows, the "dir" command doesn't
> call CreateFile() (or any other system function) for each file in the
> target directory).
As I've found out, readdir() replacement for Windows in fact gets all
the needed information (correct file size, attributes...) in
WIN32_FIND_DATA, but it just leaves it inside and returns only fields of
the dirent structure. So pg_ls_dir_files() (that calls
ReadDir()/readdir()) needs to get this information again, that leads to
opening a file on Windows.
I think it can be done more effectively by adding a new function
ReadDirExtendedInfo(), that will additionally accept a pointer to
"struct dirent_extra" with fields {valid (indicating that the structure
is filled), attributes, file size, mtime}. Maybe the advanced function
could also invoke stat() inside (not on Windows).
As to patch I proposed before, I think it's still needed to fully
support the following usage pattern:
stat();
if (no_error) {
do_something();
} else if (file_not_found) {
do_something_else();
} else {
error();
}
Best regards,
Alexander
