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 0e816386b894b033314f2b3bdae1dbcb527e10eb..94936aa47fdf5bbb159d27413920b216de5a1af5 100644 (file)
--- a/src/include/query_cache/pool_memqcache.h
+++ b/src/include/query_cache/pool_memqcache.h
@@ -285,4 +285,6 @@ extern void pool_discard_temp_query_cache(POOL_TEMP_QUERY_CACHE * temp_cache);
 extern void pool_shmem_lock(void);
 extern void pool_shmem_unlock(void);
 
+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 57750496f2b2d7e30b0477bb615af490e7d48d52..7d3c464b57ad79e6c928a0bd875b1e7652638ae0 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
@@ -3798,6 +3798,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 e8e9d28b5fa81c0e7a8859d1c05143ae63cd4dac..c2a125521e41135e0cf7207d4bacad58f2a44096 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
@@ -1991,6 +1991,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();
        {
@@ -2014,8 +2016,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
@@ -2028,7 +2041,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
@@ -2132,6 +2145,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);
@@ -2190,7 +2204,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);
@@ -2321,7 +2335,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 */
@@ -2717,7 +2730,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))
@@ -2725,7 +2738,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--;