Fix multiple query cache bug.

1) pool_add_item_shmem_cache() calls pool_init_cache_block() when
   there's no free cache item hash table entry. But this is
   unnecessary since pool_reuse_block() is already called from
   pool_add_item_shmem_cache(). This is actually harmless because the
   second pool_init_cache_block() call just set the same data as the
   first call of pool_init_cache_block(). It's just a waste of CPU
   cycle.

2) The cache blocks are supposed to be initialized while Pgpool-II
   starts up but actually not. Each cache block has the free space
   length in the block header after initialization. Since the free
   space length is not set, pool_get_block() fails to find a cache
   block which has enough free space, and it calls pool_reuse_block(),
   which is actually unnecessary (you will see something like
   "pool_reuse_block: blockid: 0" in pgpool log). Since
   pool_reuse_block() returns a free block anyway, this is just a
   waste of CPU cycle but better to fix.

Discussion: https://www.pgpool.net/pipermail/pgpool-hackers/2023-January/004259.html
Backpatch-through: 4.0

diff --git a/src/include/query_cache/pool_memqcache.h b/src/include/query_cache/pool_memqcache.h
index d51d951d2af97e8f606334247f1dc8629057b1ad..9a94710eb305a6a8e560e7f8e832ed86bacfb390 100644 (file)
--- a/src/include/query_cache/pool_memqcache.h
+++ b/src/include/query_cache/pool_memqcache.h
@@ -294,4 +294,6 @@ extern bool pool_is_shmem_lock(void);
 
 extern void InvalidateQueryCache(int tableoid, int dboid);
 
+extern void pool_init_whole_cache_blocks(void);
+
 #endif                                                 /* POOL_MEMQCACHE_H */

diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c
index c52a75500176bbea1743ac2488fb92baef9440a0..bc62d729a80e31a9635358f9848551b38d6b44a5 100644 (file)
--- a/src/main/pgpool_main.c
+++ b/src/main/pgpool_main.c
@@ -5,7 +5,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2022     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -3612,6 +3612,8 @@ initialize_shared_mem_objects(bool clear_memcache_oidmaps)
                                        (errmsg("pool_discard_oid_maps: discarded memqcache oid maps")));
 
                        pool_hash_init(pool_config->memqcache_max_num_cache);
+
+                       pool_init_whole_cache_blocks();
                }
 
 #ifdef USE_MEMCACHED

diff --git a/src/query_cache/pool_memqcache.c b/src/query_cache/pool_memqcache.c
index 7c744e6af13a2fd57731b741b6cef8a10573cdf1..3304a20df52af98f781319320d26ae7062a031c6 100644 (file)
--- a/src/query_cache/pool_memqcache.c
+++ b/src/query_cache/pool_memqcache.c
@@ -3,7 +3,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2022     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -2103,6 +2103,8 @@ pool_clear_memory_cache(void)
                pool_discard_oid_maps();
 
                pool_hash_reset(pool_config->memqcache_max_num_cache);
+
+               pool_init_whole_cache_blocks();
        }
        PG_CATCH();
        {
@@ -2126,8 +2128,19 @@ pool_memory_cache_address(void)
 }
 
 /*
- * Initialize new block
+ * Initialize whole cache blocks
  */
+void
+pool_init_whole_cache_blocks(void)
+{
+       int             blocks = pool_get_memqcache_blocks();
+       int             i;
+
+       for (i = 0; i < blocks; i++)
+       {
+               pool_init_cache_block(i);
+       }
+}
 
 /*
  * Free space management map
@@ -2140,7 +2153,7 @@ pool_memory_cache_address(void)
  * For example, if the value is 2, the free space can be between 64
  * bytes and 95 bytes.
  *
- * value free space(in bytes)
+ * value free space (in bytes)
  * 0     0-31
  * 1     32-63
  * 2     64-95
@@ -2244,6 +2257,7 @@ static POOL_CACHE_BLOCKID pool_reuse_block(void)
        reused_block = *pool_fsmm_clock_hand;
        p = block_address(reused_block);
 
+       /* Remove all items in this block from hash table */
        for (i = 0; i < bh->num_items; i++)
        {
                cip = item_pointer(p, i);
@@ -2302,7 +2316,7 @@ static POOL_CACHE_BLOCKID pool_get_block(size_t free_space)
                if (p[i] >= encode_value)
                {
                        /*
-                        * This block *may" have enough space. We need to make sure it
+                        * This block may not have enough space. We need to make sure it
                         * actually has enough space.
                         */
                        bh = (POOL_CACHE_BLOCK_HEADER *) block_address(i);
@@ -2433,7 +2447,6 @@ static POOL_CACHEID * pool_add_item_shmem_cache(POOL_QUERY_HASH * query_hash, ch
        {
                /* If not, reuse next victim block */
                blockid = pool_reuse_block();
-               pool_init_cache_block(blockid);
        }
 
        /* Get block address on shmem */
@@ -2830,7 +2843,7 @@ pool_delete_item_shmem_cache(POOL_CACHEID * cacheid)
        pool_hash_delete(&key);
 
        /*
-        * If the deleted item is last one in the block, we add it to the free
+        * If the deleted item is the last one in the block, we add it to the free
         * space.
         */
        if (cacheid->itemid == (bh->num_items - 1))
@@ -2838,7 +2851,7 @@ pool_delete_item_shmem_cache(POOL_CACHEID * cacheid)
                bh->free_bytes += size;
                ereport(DEBUG1,
                                (errmsg("memcache deleting item data"),
-                                errdetail("deleted %d bytes, freebytes is = %d",
+                                errdetail("deleted %d bytes, freebytes is %d",
                                                   size, bh->free_bytes)));
 
                bh->num_items--;