On Sat, 2013-05-25 at 13:55 -0500, Jon Nelson wrote:
> Ack. I've revised the patch to always have the GUC (for now), default
> to false, and if configure can't find posix_fallocate (or the user
> disables it by way of pg_config_manual.h) then it remains a GUC that
> simply can't be changed.
Why have a GUC here at all? Perhaps this was already discussed, and I
missed it? Is it just for testing purposes, or did you intend for it to
be in the final version?
If it's supported, it seems like we always want it. I doubt there are
cases where it hurts performance; but if there are, it's pretty hard for
a DBA to know what those cases are, anyway.
Also:
* The other code assumes that no errno means ENOSPC. We should be
consistent about that assumption, and do the same thing in the fallocate
case.
* You check for the presence of posix_fallocate at configure time, but
don't #ifdef the call site. It looks like you removed this from the v2
patch, was there a reason for that? Won't that cause build errors for
platforms without it?
I started looking at this patch and it looks like we are getting a
consensus that it's the right approach. Microbenchmarks appear to show a
benefit, and (thanks to Noah's comment) it seems like the change is
safe. Are there any remaining questions or objections?
Regards,Jeff Davis