Tom Lane wrote:
"Heikki Linnakangas" <[EMAIL PROTECTED]> writes:
If we're going to check for file length, we should definitely check the
file length when we write it, so that we fail at PREPARE time, and not
at COMMIT time.
I think this is mere self-delusion, unfortunately. You can never be
certain at prepare time that a large alloc will succeed sometime later
in a different process.
Gavin's complaint is essentially that a randomly chosen hard limit is
bad, and I agree with that. Choosing a larger hard limit doesn't make
it less random.
It might be worth checking at prepare that the file size doesn't exceed
MaxAllocSize, but any smaller limit strikes me as (a) unnecessarily
restrictive and (b) not actually creating any useful guarantee.
Hmm, I guess you're right.
Patch attached. I can't commit it myself right now, but will do so as
soon as I can, unless there's objections.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/access/transam/twophase.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/transam/twophase.c,v
retrieving revision 1.42
diff -c -r1.42 twophase.c
*** src/backend/access/transam/twophase.c 12 May 2008 00:00:45 -0000 1.42
--- src/backend/access/transam/twophase.c 16 May 2008 09:56:56 -0000
***************
*** 56,61 ****
--- 56,62 ----
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
+ #include "utils/memutils.h"
/*
***************
*** 866,871 ****
--- 867,881 ----
hdr->total_len = records.total_len + sizeof(pg_crc32);
/*
+ * If the file size exceeds MaxAllocSize, we won't be able to read it in
+ * ReadTwoPhaseFile. Check for that now, rather than fail at commit time.
+ */
+ if (hdr->total_len > MaxAllocSize)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("two-phase state file maximum length exceeed")));
+
+ /*
* Create the 2PC state file.
*
* Note: because we use BasicOpenFile(), we are responsible for ensuring
***************
*** 1044,1051 ****
}
/*
! * Check file length. We can determine a lower bound pretty easily. We
! * set an upper bound mainly to avoid palloc() failure on a corrupt file.
*/
if (fstat(fd, &stat))
{
--- 1054,1060 ----
}
/*
! * Check file length. We can determine a lower bound pretty easily.
*/
if (fstat(fd, &stat))
{
***************
*** 1059,1066 ****
if (stat.st_size < (MAXALIGN(sizeof(TwoPhaseFileHeader)) +
MAXALIGN(sizeof(TwoPhaseRecordOnDisk)) +
! sizeof(pg_crc32)) ||
! stat.st_size > 10000000)
{
close(fd);
return NULL;
--- 1068,1074 ----
if (stat.st_size < (MAXALIGN(sizeof(TwoPhaseFileHeader)) +
MAXALIGN(sizeof(TwoPhaseRecordOnDisk)) +
! sizeof(pg_crc32)))
{
close(fd);
return NULL;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
