View Issue Details

IDProjectCategoryView StatusLast Update
0000070Pgpool-IIBugpublic2013-10-19 14:01
ReporterglenngerhardtAssigned Tot-ishii 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionopen 
PlatformOSCentosOS Version6.3
Product Version 
Target VersionFixed in Version 
Summary0000070: on memory caching can fill up, expired items not purged.
DescriptionWhen the number of elements that can be cached is depleted, caching of new
items will cease. There are multiple expired cache items that could
be purged to make room for the new cache items, but purging of expired
items only occurs when free memory is depleted.

Increasing the value of memqcache_max_num_cache may help, but if this
value is reached prior to reaching memqcache_total_size then caching
of new entries will stop working and purging of old items will never happen.

pool_hash_insert() will check to see if enough memory is available to
insert the item. There is enough memory, so there is no purge of
old items in the cache. Then later in pool_hash_insert() it will
call get_new_has_element() which will fail due to being out of elements.
This log is seen at that time:
pool_hash_insert: could not get new element

To fix this: purge old expired cache entries when memqcache_max_num_cache
is hit.
Steps To ReproduceEasily reproduced when you lower the memqcache_max_num_cache value to a small
number such as 1024, but keep the memqcache_total_size a large value.
Populate the cache with items. When it attemps to cache the 1025th item,
it will fail. As the 1024 items in cache expire, they are never purged out.
I've seen the scenario occur with higher values, but lowering this
makes repro quicker.

Running with the inverse config (large memqcache_max_num and small
memqcache_total_size) will show that expired cache items are purged to make
room when free memory is depleted.
Additional Informationpgpool.conf parameters:

memory_cache_enabled = on
memqcache_method = 'shmem'
memqcache_total_size = 1073741824
memqcache_max_num_cache = 1048576
memqcache_expire = 10
memqcache_auto_cache_invalidation = on
memqcache_maxcache = 409600
memqcache_cache_block_size = 1048576
white_memqcache_table_list = ''
black_memqcache_table_list = ''
memqcache_oiddir = '/dev/shm/pgpool/oiddir'
TagsNo tags attached.

Activities

t-ishii

2013-09-04 13:36

developer   ~0000319

You are right. Before insert a new cache entry, pgpool-II should make sure that there's at least one usable hash element. Included patch should do the trick. Note that it does not purge expired cache entry, rather look for next victim cache block because: 1) cache expiration is not always on 2) look for expired cache entry could be very expensive (there could be huge number of cache entries).

Note that the patch is for git master head. But should be applied to V3.3 or V3.2 with something like: patch -p 3

t-ishii

2013-09-04 13:36

developer  

poool_memqcache.c.patch (1,438 bytes)
diff --git a/src/query_cache/pool_memqcache.c b/src/query_cache/pool_memqcache.c
index e65b01f..a0eabcf 100644
--- a/src/query_cache/pool_memqcache.c
+++ b/src/query_cache/pool_memqcache.c
@@ -115,6 +115,7 @@ static int pool_hash_insert(POOL_QUERY_HASH *key, POOL_CACHEID *cacheid, bool up
 static uint32 create_hash_key(POOL_QUERY_HASH *key);
 static volatile POOL_HASH_ELEMENT *get_new_hash_element(void);
 static void put_back_hash_element(volatile POOL_HASH_ELEMENT *element);
+static bool is_free_hash_element(void);
 static char *get_relation_without_alias(RangeVar *relation);
 
 /*
@@ -2002,6 +2003,16 @@ static POOL_CACHEID *pool_add_item_shmem_cache(POOL_QUERY_HASH *query_hash, char
 	 */
 	pool_init_cache_block(blockid);
 
+	/*
+	 * Make sure that we have at least one free hash element.
+	 */
+	while (!is_free_hash_element())
+	{
+		/* If not, reuse next victim block */
+		blockid = pool_reuse_block();
+		pool_init_cache_block(blockid);
+	}
+
 	/* Get block address on shmem */
 	p = block_address(blockid);
 	bh = (POOL_CACHE_BLOCK_HEADER *)p;
@@ -3709,6 +3720,14 @@ static void put_back_hash_element(volatile POOL_HASH_ELEMENT *element)
 }
 
 /*
+ * Return true if there's a free hash element.
+ */
+static bool is_free_hash_element(void)
+{
+	return hash_free->next != NULL;
+}
+
+/*
  * Returns shared memory cache stats.
  * Subsequent call to this function will break return value
  * because its in static memory.
poool_memqcache.c.patch (1,438 bytes)

glenngerhardt

2013-10-17 07:07

reporter   ~0000347

This change appears to be working.

Issue History

Date Modified Username Field Change
2013-09-04 00:03 glenngerhardt New Issue
2013-09-04 09:48 t-ishii Assigned To => t-ishii
2013-09-04 09:48 t-ishii Status new => assigned
2013-09-04 13:36 t-ishii Note Added: 0000319
2013-09-04 13:36 t-ishii File Added: poool_memqcache.c.patch
2013-09-04 13:36 t-ishii Status assigned => feedback
2013-10-17 07:07 glenngerhardt Note Added: 0000347
2013-10-17 07:07 glenngerhardt Status feedback => assigned
2013-10-19 13:18 t-ishii Status assigned => resolved
2013-10-19 14:01 t-ishii Changeset attached => pgpool2 master d6c447df