From c394bd331a701af06481e6e61a76ea82b18965eb Mon Sep 17 00:00:00 2001 From: Andrew Dunstan <andrew@dunslane.net> Date: Wed, 11 Mar 2009 03:33:29 +0000 Subject: [PATCH] Use thread-local storage for querybuffer in fmtId() on Windows, when needed (i.e. when 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. --- src/bin/pg_dump/dumputils.c | 59 +++++++++++++++++++++++++--- src/bin/pg_dump/dumputils.h | 3 +- src/bin/pg_dump/pg_backup_archiver.c | 22 ++++++++--- src/bin/pg_dump/pg_restore.c | 5 ++- 4 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 85b3373c535..1f9d6832c82 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.44 2009/01/22 20:16:07 tgl Exp $ + * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.c,v 1.45 2009/03/11 03:33:29 adunstan Exp $ * *------------------------------------------------------------------------- */ @@ -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 b8a3532fa88..952c8b3653e 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.h,v 1.23 2009/01/22 20:16:07 tgl Exp $ + * $PostgreSQL: pgsql/src/bin/pg_dump/dumputils.h,v 1.24 2009/03/11 03:33:29 adunstan Exp $ * *------------------------------------------------------------------------- */ @@ -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 95564bccd20..8cd171c2a90 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -15,7 +15,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.165 2009/03/05 14:51:10 petere Exp $ + * $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.166 2009/03/11 03:33:29 adunstan Exp $ * *------------------------------------------------------------------------- */ @@ -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 02b81b8e8a8..c557f9706fe 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -34,12 +34,13 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/bin/pg_dump/pg_restore.c,v 1.94 2009/02/26 16:02:38 petere Exp $ + * $PostgreSQL: pgsql/src/bin/pg_dump/pg_restore.c,v 1.95 2009/03/11 03:33:29 adunstan Exp $ * *------------------------------------------------------------------------- */ #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]); -- GitLab