Fix query cache.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Sat, 24 Sep 2022 08:00:12 +0000 (17:00 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Sat, 24 Sep 2022 08:00:12 +0000 (17:00 +0900)
The commit dc559c07ee5affc7035efa6e0f00185e211079a0 introduced shared
lock by using flock(2).  However it opened lock file in parent process
then the file descriptor was shared by child process. This is
wrong. The lock file needs to be opened by each child process.

src/include/pool.h
src/include/query_cache/pool_memqcache.h
src/main/pgpool_main.c
src/query_cache/pool_memqcache.c

index e37380d661d358084c71deeec7925d45f16f7635..af07069739a1fdb1ad62e755734dc8f90acf8144 100644 (file)
@@ -83,6 +83,9 @@
 /* status file name */
 #define STATUS_FILE_NAME "pgpool_status"
 
+/* query cache lock file name */
+#define QUERY_CACHE_LOCK_FILE "memq_lock_file"
+
 /* default string used to identify pgpool on syslog output */
 #define DEFAULT_SYSLOG_IDENT "pgpool"
 
index a02410d53b03c3417874d4a8de8ddcc1e1daa971..ef9d6a102388a5f64dba32ee24ccfd477f08e848 100644 (file)
@@ -235,12 +235,6 @@ typedef enum
        POOL_MEMQ_EXCLUSIVE_LOCK,
 } POOL_MEMQ_LOCK_TYPE;
 
-/*
- * File descriptor used for locking in query cache.
- * Inherited to child process.
- */
-extern int memq_lock_fd;
-
 extern int     pool_hash_init(int nelements);
 extern size_t pool_hash_size(int nelements);
 extern POOL_CACHEID * pool_hash_search(POOL_QUERY_HASH * key);
index 30f7857002ce8635b9661524c9372fdf58b662bd..5dde03646fac634a92d7f87008f7a7c6244ff83b 100644 (file)
@@ -272,12 +272,6 @@ static int dummy_status;
  */
 volatile SI_ManageInfo *si_manage_info;
 
-/*
- * File descriptor used for locking in query cache.
- * Inherited to child process.
- */
-int    memq_lock_fd;
-
 /*
 * pgpool main program
 */
@@ -494,6 +488,26 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps)
                free(inet_fds);
        }
 
+       /* For query cache concurrency control */
+       if (pool_config->memory_cache_enabled)
+       {
+               char path[1024];
+               int             lfd;
+
+               snprintf(path, sizeof(path), "%s/QUERY_CACHE_LOCK_FILE", pool_config->logdir);
+               lfd = open(path, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR);
+               if (lfd == -1)
+               {
+                       ereport(FATAL,
+                                       (errmsg("Failed to open lock file for query cache \"%s\"", path),
+                                        errdetail("%m")));
+               }
+               close(lfd);
+
+               /* Register file unlink at exit */
+               on_proc_exit(FileUnlink, (Datum) path);
+       }
+
        /*
         * We need to block signal here. Otherwise child might send some signals,
         * for example SIGUSR1(fail over).  Children will inherit signal blocking
@@ -591,24 +605,6 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps)
        /* Create or write status file */
        (void) write_status_file();
 
-       /* For query cache concurrency control */
-       if (pool_config->memory_cache_enabled)
-       {
-               char path[1024];
-
-               snprintf(path, sizeof(path), "%s/memq_lock_file", pool_config->logdir);
-               memq_lock_fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR);
-               if (memq_lock_fd == -1)
-               {
-                       ereport(FATAL,
-                                       (errmsg("Failed to open lock file for query cache \"%s\"", path),
-                                        errdetail("%m")));
-               }
-
-               /* Register file unlink at exit */
-               on_proc_exit(FileUnlink, (Datum) path);
-       }
-
        /* This is the main loop */
        for (;;)
        {
index 6f90c6393f887dcb4ed196b20ca050e4e53af10b..e5e3b3061bb6270d7dcf42f25015d92320e29739 100644 (file)
@@ -2942,12 +2942,33 @@ pool_wipe_out_cache_block(POOL_CACHE_BLOCKID blockid)
 }
 #endif
 
+#undef LOCK_TRACE
+
+static int memq_lock_fd = 0;
+
 /*
  * Acquire lock: XXX giant lock
  */
 void
 pool_shmem_lock(POOL_MEMQ_LOCK_TYPE type)
 {
+       if (memq_lock_fd == 0)
+       {
+               char path[1024];
+
+               snprintf(path, sizeof(path), "%s/%s", pool_config->logdir, QUERY_CACHE_LOCK_FILE);
+               memq_lock_fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR);
+               if (memq_lock_fd == -1)
+               {
+                       ereport(FATAL,
+                                       (errmsg("Failed to open lock file for query cache \"%s\"", path),
+                                        errdetail("%m")));
+               }
+       }
+
+#ifdef LOCK_TRACE
+               elog(LOG, "LOCK TRACE: try to aquire lock %s", type == POOL_MEMQ_EXCLUSIVE_LOCK? "LOCK_EX" : "LOCK_SH");
+#endif
        if (pool_is_shmem_cache() && !is_shmem_locked)
        {
                if (flock(memq_lock_fd, type == POOL_MEMQ_EXCLUSIVE_LOCK? LOCK_EX : LOCK_SH))
@@ -2955,7 +2976,12 @@ pool_shmem_lock(POOL_MEMQ_LOCK_TYPE type)
                        ereport(FATAL,
                                        (errmsg("Failed to lock file for query cache"),
                                         errdetail("%m")));
+
                }
+
+#ifdef LOCK_TRACE
+               elog(LOG, "LOCK TRACE: aquire lock %s", type == POOL_MEMQ_EXCLUSIVE_LOCK? "LOCK_EX" : "LOCK_SH");
+#endif
                is_shmem_locked = true;
        }
 }
@@ -2974,6 +3000,9 @@ pool_shmem_unlock(void)
                                        (errmsg("Failed to unlock file for query cache"),
                                         errdetail("%m")));
                }
+#ifdef LOCK_TRACE
+               elog(LOG, "LOCK TRACE: unlock");
+#endif
                is_shmem_locked = false;
        }
 }