Re: pgsql: Improve error handling in RemovePgTempFiles().

Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Perhaps RemovePgTempFiles() could check if each one exists before
> calling RemovePgTempFilesInDir(), like in the attached?  Alternatively
> we could make RemovePgTempFilesInDir() return early if temp_dir ==
> NULL (going against your commit message above), or I suppose we could
> arrange for temporary directories always to exist in base and each
> tablespace.

After further thought I don't especially like your solution as written:
it has a race condition if the directory is deleted between the pre-check
and the AllocateDir proper.  What seems better to me is a tighter version
of your second alternative: let's add a "missing_ok" parameter to
RemovePgTempFilesInDir, which'd be passed as true only at the top level,
and do something like

    temp_dir = AllocateDir(tmpdirname);

+    if (temp_dir == NULL && errno == ENOENT && missing_ok)
+        return;
+
    while ((temp_de = ReadDirExtended(temp_dir, tmpdirname, LOG)) != NULL)

This might be problematic if RemovePgTempFilesInDir were a globally
exposed API, but it isn't, so adding a parameter seems fine.

Hmmm ... actually, in the recursive call case, it wouldn't be that
awful to ignore ENOENT either; if a directory goes away between being
stat'd and being opened, you'd still get a log message about rmdir
failure at the caller level.  So maybe we should just do your
second alternative as-is (ie, code as above but without missing_ok).
Having an explicit control parameter seems marginally clearer but
I'm not sure it's buying anything meaningful.  Thoughts?

            regards, tom lane