Re: pgsql: Improve error handling in RemovePgTempFiles(). - Mailing list pgsql-committers

From Tom Lane
Subject Re: pgsql: Improve error handling in RemovePgTempFiles().
Date
Msg-id 17683.1515343346@sss.pgh.pa.us
Whole thread Raw
In response to Re: pgsql: Improve error handling in RemovePgTempFiles().  (Thomas Munro <thomas.munro@enterprisedb.com>)
Responses Re: pgsql: Improve error handling in RemovePgTempFiles().
List pgsql-committers
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


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: Improve error handling in RemovePgTempFiles().
Next
From: Tom Lane
Date:
Subject: pgsql: Back off chattiness in RemovePgTempFiles().