Re: Re: patch review : Add ability to constrain backend temporary file space - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Re: patch review : Add ability to constrain backend temporary file space
Date
Msg-id 386.1310871630@sss.pgh.pa.us
Whole thread Raw
In response to Re: Re: patch review : Add ability to constrain backend temporary file space  (Mark Kirkwood <mark.kirkwood@catalyst.net.nz>)
List pgsql-hackers
Mark Kirkwood <mark.kirkwood@catalyst.net.nz> writes:
> This  version moves the check *before* we write the new buffer, so 
> should take care of issues about really large write buffers, plugins 
> etc.

This logic seems pretty obviously wrong:

+     if (temp_file_limit >= 0 && VfdCache[file].fdstate & FD_TEMPORARY)
+     {
+         if (VfdCache[file].seekPos + amount >= VfdCache[file].fileSize)
+         {
+             increaseSize = true;
+             if ((temporary_files_size + (double)amount) / 1024.0 > (double)temp_file_limit)
+                 ereport(ERROR,
+                         (errcode(ERRCODE_QUERY_CANCELED),
+                         errmsg("aborting due to exceeding temp file limit")));
+         }
+     }

It's supposing that the write length ("amount") is exactly the amount by
which the file length will grow, which would only be true if seekPos is
exactly equal to fileSize before the write.  I think that would often be
the case in our usage patterns, but surely code at this low level should
not be assuming that.

BTW, the definition of the GUC seems to constrain the total temp file
size to be no more than 2GB on 32-bit machines ... do we really want
it to act like that?
        regards, tom lane


pgsql-hackers by date:

Previous
From: "David E. Wheeler"
Date:
Subject: Re: pg_class.relistemp
Next
From: Tom Lane
Date:
Subject: Re: Re: patch review : Add ability to constrain backend temporary file space