Message-ID: <4DA2BCC2.7080903@enterprisedb.com>
Date: Mon, 11 Apr 2011 11:33:06 +0300
From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; fi-FI;
	rv:1.9.1.16) Gecko/20110307 Icedove/3.0.11
MIME-Version: 1.0
To: Kevin Grittner <Kevin.Grittner@wicourts.gov>
CC: Dan Ports <drkp@csail.mit.edu>, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>,
	pgsql-hackers@postgresql.org
Subject: Re: SSI bug?
References: <4D8F54E6020000250003BD16@gw.wicourts.gov>
	<4D8F54E6020000250003BD16@gw.wicourts.gov>
	<20110328031616.CDAA719CEDF@mail.netbsd.org>
	<4D943BEC020000250003BFDA@gw.wicourts.gov>
	<4D94A2BB.3090605@enterprisedb.com>
	<4D946036020000250003BFFC@gw.wicourts.gov>
	<20110331181216.GF81592@csail.mit.edu>
	<4D948066020000250003C00B@gw.wicourts.gov>
	<4D94C889.3050607@enterprisedb.com>
	<4D948A7D020000250003C012@gw.wicourts.gov>
In-Reply-To: <4D948A7D020000250003C012@gw.wicourts.gov>
Content-Type: multipart/mixed; boundary="------------030207010707010304070103"

This is a multi-part message in MIME format.
--------------030207010707010304070103
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

On 31.03.2011 22:06, Kevin Grittner wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  wrote:
>
>> That's not enough. The hash tables can grow beyond the maximum
>> size you specify in ShmemInitHash. It's just a hint to size the
>> directory within the hash table.
>>
>> We'll need to teach dynahash not to allocate any more entries
>> after the preallocation. A new HASH_NO_GROW flag to hash_create()
>> seems like a suitable interface.
>
> OK.  If we're doing that, is it worth taking a look at the "safety
> margin" added to the size calculations, and try to make the
> calculations more accurate?
>
> Would you like me to code a patch for this?

I finally got around to look at this. Attached patch adds a 
HASH_FIXED_SIZE flag, which disables the allocation of new entries after 
the initial allocation. I believe we have consensus to make the 
predicate lock hash tables fixed-size, so that there's no competition of 
the slack shmem space between predicate lock structures and the regular 
lock maanager.

I also noticed that there's a few hash_search(HASH_ENTER) calls in 
predicate.c followed by check for a NULL result. But with HASH_ENTER, 
hash_search never returns NULL, it throws an "out of shared memory" 
error internally. I changed those calls to use HASH_ENTER_NULL, so you 
now get the intended error message with the hint to raise 
max_pred_locks_per_transaction.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

--------------030207010707010304070103
Content-Type: text/x-diff;
 name="fixed-size-shmem-3.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="fixed-size-shmem-3.patch"

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 401acdb..6ff41fc 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -959,17 +959,15 @@ InitPredicateLocks(void)
 {
 	HASHCTL		info;
 	int			hash_flags;
-	long		init_table_size,
-				max_table_size;
+	long		max_table_size;
 	Size		requestSize;
 	bool		found;
 
 	/*
-	 * Compute init/max size to request for predicate lock target hashtable.
+	 * Compute size of predicate lock target hashtable.
 	 * Note these calculations must agree with PredicateLockShmemSize!
 	 */
 	max_table_size = NPREDICATELOCKTARGETENTS();
-	init_table_size = max_table_size / 2;
 
 	/*
 	 * Allocate hash table for PREDICATELOCKTARGET structs.  This stores
@@ -980,17 +978,16 @@ InitPredicateLocks(void)
 	info.entrysize = sizeof(PREDICATELOCKTARGET);
 	info.hash = tag_hash;
 	info.num_partitions = NUM_PREDICATELOCK_PARTITIONS;
-	hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
+	hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION | HASH_FIXED_SIZE);
 
 	PredicateLockTargetHash = ShmemInitHash("PREDICATELOCKTARGET hash",
-											init_table_size,
+											max_table_size,
 											max_table_size,
 											&info,
 											hash_flags);
 
 	/* Assume an average of 2 xacts per target */
 	max_table_size *= 2;
-	init_table_size *= 2;
 
 	/*
 	 * Reserve an entry in the hash table; we use it to make sure there's
@@ -1011,18 +1008,17 @@ InitPredicateLocks(void)
 	info.entrysize = sizeof(PREDICATELOCK);
 	info.hash = predicatelock_hash;
 	info.num_partitions = NUM_PREDICATELOCK_PARTITIONS;
-	hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
+	hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION | HASH_FIXED_SIZE);
 
 	PredicateLockHash = ShmemInitHash("PREDICATELOCK hash",
-									  init_table_size,
+									  max_table_size,
 									  max_table_size,
 									  &info,
 									  hash_flags);
 
 	/*
-	 * Compute init/max size to request for serializable transaction
-	 * hashtable. Note these calculations must agree with
-	 * PredicateLockShmemSize!
+	 * Compute size for serializable transaction hashtable.
+	 * Note these calculations must agree with PredicateLockShmemSize!
 	 */
 	max_table_size = (MaxBackends + max_prepared_xacts);
 
@@ -1093,7 +1089,7 @@ InitPredicateLocks(void)
 	info.keysize = sizeof(SERIALIZABLEXIDTAG);
 	info.entrysize = sizeof(SERIALIZABLEXID);
 	info.hash = tag_hash;
-	hash_flags = (HASH_ELEM | HASH_FUNCTION);
+	hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_FIXED_SIZE);
 
 	SerializableXidHash = ShmemInitHash("SERIALIZABLEXID hash",
 										max_table_size,
@@ -2045,7 +2041,7 @@ CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
 	target = (PREDICATELOCKTARGET *)
 		hash_search_with_hash_value(PredicateLockTargetHash,
 									targettag, targettaghash,
-									HASH_ENTER, &found);
+									HASH_ENTER_NULL, &found);
 	if (!target)
 		ereport(ERROR,
 				(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -2060,7 +2056,7 @@ CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
 	lock = (PREDICATELOCK *)
 		hash_search_with_hash_value(PredicateLockHash, &locktag,
 			PredicateLockHashCodeFromTargetHashCode(&locktag, targettaghash),
-									HASH_ENTER, &found);
+									HASH_ENTER_NULL, &found);
 	if (!lock)
 		ereport(ERROR,
 				(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -3251,7 +3247,7 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
 			predlock = hash_search_with_hash_value(PredicateLockHash, &tag,
 								PredicateLockHashCodeFromTargetHashCode(&tag,
 															  targettaghash),
-												   HASH_ENTER, &found);
+												   HASH_ENTER_NULL, &found);
 			if (!predlock)
 				ereport(ERROR,
 						(errcode(ERRCODE_OUT_OF_MEMORY),
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index f718785..d902729 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -160,6 +160,7 @@ struct HTAB
 	MemoryContext hcxt;			/* memory context if default allocator used */
 	char	   *tabname;		/* table name (for error messages) */
 	bool		isshared;		/* true if table is in shared memory */
+	bool		isfixed;		/* if true, don't enlarge */
 
 	/* freezing a shared table isn't allowed, so we can keep state here */
 	bool		frozen;			/* true = no more inserts allowed */
@@ -435,6 +436,8 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
 					 errmsg("out of memory")));
 	}
 
+	if (flags & HASH_FIXED_SIZE)
+		hashp->isfixed = true;
 	return hashp;
 }
 
@@ -1334,6 +1337,9 @@ element_alloc(HTAB *hashp, int nelem)
 	HASHELEMENT *prevElement;
 	int			i;
 
+	if (hashp->isfixed)
+		return false;
+
 	/* Each element has a HASHELEMENT header plus user data. */
 	elementSize = MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(hctlv->entrysize);
 
diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index bca34d2..ce54c0a 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -92,6 +92,7 @@ typedef struct HASHCTL
 #define HASH_CONTEXT	0x200	/* Set memory allocation context */
 #define HASH_COMPARE	0x400	/* Set user defined comparison function */
 #define HASH_KEYCOPY	0x800	/* Set user defined key-copying function */
+#define HASH_FIXED_SIZE	0x1000	/* Initial size is a hard limit */
 
 
 /* max_dsize value to indicate expansible directory */

--------------030207010707010304070103--