Use thread-local storage for querybuffer in fmtId() on Windows, when needed (i.e...

running pg_restore, which might run in parallel).
Only reopen archive file when we really need to read from it, in parallel code. Otherwise,
close it immediately in a worker, if possible.

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index baa185fd1c583d57161224b449c13992c0b93ff7..1facfa6cb458bdc6f6a487be6c5bf8ed5ea75b53 100644 (file)
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -31,25 +31,72 @@ static char *copyAclUserName(PQExpBuffer output, char *input);
 static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
                                   const char *subname);
 
+#ifdef WIN32
+static bool parallel_init_done = false;
+static DWORD tls_index;
+#endif
+
+void
+init_parallel_dump_utils(void)
+{
+#ifdef WIN32
+       if (! parallel_init_done)
+       {
+               tls_index = TlsAlloc();
+               parallel_init_done = true;
+       }
+#endif
+}
 
 /*
- *     Quotes input string if it's not a legitimate SQL identifier as-is.
+ *  Quotes input string if it's not a legitimate SQL identifier as-is.
  *
- *     Note that the returned string must be used before calling fmtId again,
- *     since we re-use the same return buffer each time.  Non-reentrant but
- *     avoids memory leakage.
+ *  Note that the returned string must be used before calling fmtId again,
+ *  since we re-use the same return buffer each time.  Non-reentrant but
+ *  reduces memory leakage. (On Windows the memory leakage will be one buffer
+ *  per thread, which is at least better than one per call).
  */
 const char *
 fmtId(const char *rawid)
 {
-       static PQExpBuffer id_return = NULL;
+       /* 
+        * The Tls code goes awry if we use a static var, so we provide for both
+        * static and auto, and omit any use of the static var when using Tls.
+        */
+       static PQExpBuffer s_id_return = NULL;
+       PQExpBuffer id_return;
+
        const char *cp;
        bool            need_quotes = false;
 
+#ifdef WIN32
+       if (parallel_init_done)
+               id_return = (PQExpBuffer) TlsGetValue(tls_index); /* 0 when not set */
+       else
+               id_return = s_id_return;
+#else
+       id_return = s_id_return;
+#endif
+
        if (id_return)                          /* first time through? */
+       {
+               /* same buffer, just wipe contents */
                resetPQExpBuffer(id_return);
+       }
        else
+       {
+               /* new buffer */
                id_return = createPQExpBuffer();
+#ifdef WIN32           
+               if (parallel_init_done)
+                       TlsSetValue(tls_index,id_return);
+               else
+                       s_id_return = id_return;
+#else
+               s_id_return = id_return;
+#endif
+               
+       }
 
        /*
         * These checks need to match the identifier production in scan.l. Don't

diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 97f2ad6621cd6209549fea0f2b4e2960c21ca980..f493a41576974c80a054951d4f387c3c0e49fc40 100644 (file)
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -19,6 +19,7 @@
 #include "libpq-fe.h"
 #include "pqexpbuffer.h"
 
+extern void init_parallel_dump_utils(void);
 extern const char *fmtId(const char *identifier);
 extern void appendStringLiteral(PQExpBuffer buf, const char *str,
                                        int encoding, bool std_strings);

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index b70673c05753ccfd46ba0e6b938719fb7f4f9106..cb806d9c573e5137e3d8be6db12dc27c8ba2ce48 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3467,12 +3467,20 @@ parallel_restore(RestoreArgs *args)
 
        /*
         * Close and reopen the input file so we have a private file pointer
-        * that doesn't stomp on anyone else's file pointer.
+        * that doesn't stomp on anyone else's file pointer, if we're actually 
+        * going to need to read from the file. Otherwise, just close it
+        * except on Windows, where it will possibly be needed by other threads.
         *
-        * Note: on Windows, since we are using threads not processes, this
-        * *doesn't* close the original file pointer but just open a new one.
+        * Note: on Windows, since we are using threads not processes, the
+        * reopen call *doesn't* close the original file pointer but just open 
+        * a new one.
         */
-       (AH->ReopenPtr) (AH);
+       if (te->section == SECTION_DATA )
+               (AH->ReopenPtr) (AH);
+#ifndef WIN32
+       else
+               (AH->ClosePtr) (AH);
+#endif
 
        /*
         * We need our own database connection, too
@@ -3490,7 +3498,9 @@ parallel_restore(RestoreArgs *args)
        PQfinish(AH->connection);
        AH->connection = NULL;
 
-       (AH->ClosePtr) (AH);
+       /* If we reopened the file, we are done with it, so close it now */
+       if (te->section == SECTION_DATA )
+               (AH->ClosePtr) (AH);
 
        if (retval == 0 && AH->public.n_errors)
                retval = WORKER_IGNORED_ERRORS;

diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index d1ffcff363fc5515e9c3d65b401a196a530d69ef..94fe791570c07f6f6f10cf285b7f763880e9548d 100644 (file)
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -40,6 +40,7 @@
  */
 
 #include "pg_backup_archiver.h"
+#include "dumputils.h"
 
 #include <ctype.h>
 
@@ -125,6 +126,8 @@ main(int argc, char **argv)
 
        set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
 
+       init_parallel_dump_utils();
+
        opts = NewRestoreOptions();
 
        progname = get_progname(argv[0]);