The attached patch fixes two issues with parallel restore:

   * the static buffer problem in dumputils.c::fmtId() on Windows
     (solution: use thread-local storage)
   * ReopenPtr() is called too often


There is one remaining bug I know of that I can reproduce: we can get into deadlock when two tables are foreign keyed to each other. So I need to get a bit more paranoid about dependencies.

I can't reproduce Olivier Prennant's file closing problem on Unixware. Is it still happening after application of this patch?

cheers

andrew
Index: src/bin/pg_dump/dumputils.c
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.44
diff -c -r1.44 dumputils.c
*** src/bin/pg_dump/dumputils.c	22 Jan 2009 20:16:07 -0000	1.44
--- src/bin/pg_dump/dumputils.c	9 Mar 2009 22:33:32 -0000
***************
*** 31,36 ****
--- 31,50 ----
  static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
  				   const char *subname);
  
+ #ifdef WIN32
+ static DWORD tls_index;
+ #endif
+ 
+ void
+ init_dump_utils()
+ {
+ #ifdef WIN32
+ 	/* reserve one thread-local slot for the fmtId query buffer address */
+ 	tls_index = TlsAlloc();
+ #endif
+ }
+ 
+ 
  
  /*
   *	Quotes input string if it's not a legitimate SQL identifier as-is.
***************
*** 42,55 ****
--- 56,87 ----
  const char *
  fmtId(const char *rawid)
  {
+ #ifdef WIN32
+ 	PQExpBuffer id_return;
+ #else
  	static PQExpBuffer id_return = NULL;
+ #endif
  	const char *cp;
  	bool		need_quotes = false;
+ 	char *retval;
+ 
+ #ifdef WIN32
+ 	id_return = (PQExpBuffer) TlsGetValue(tls_index); /* returns 0 until set */
+ #endif
  
  	if (id_return)				/* first time through? */
+ 	{
+ 		/* same buffer, just wipe contents */
  		resetPQExpBuffer(id_return);
+ 	}
  	else
+ 	{
+ 		/* new buffer */
  		id_return = createPQExpBuffer();
+ #ifdef WIN32		
+ 		TlsSetValue(tls_index,id_return);
+ #endif
+ 	}
  
  	/*
  	 * These checks need to match the identifier production in scan.l. Don't
***************
*** 111,117 ****
  		appendPQExpBufferChar(id_return, '\"');
  	}
  
! 	return id_return->data;
  }
  
  
--- 143,151 ----
  		appendPQExpBufferChar(id_return, '\"');
  	}
  
! 	retval = id_return->data;
! 	return retval;
! 
  }
  
  
Index: src/bin/pg_dump/dumputils.h
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/dumputils.h,v
retrieving revision 1.23
diff -c -r1.23 dumputils.h
*** src/bin/pg_dump/dumputils.h	22 Jan 2009 20:16:07 -0000	1.23
--- src/bin/pg_dump/dumputils.h	9 Mar 2009 22:33:32 -0000
***************
*** 19,24 ****
--- 19,25 ----
  #include "libpq-fe.h"
  #include "pqexpbuffer.h"
  
+ extern void init_dump_utils(void);
  extern const char *fmtId(const char *identifier);
  extern void appendStringLiteral(PQExpBuffer buf, const char *str,
  					int encoding, bool std_strings);
Index: src/bin/pg_dump/pg_backup_archiver.c
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.165
diff -c -r1.165 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c	5 Mar 2009 14:51:10 -0000	1.165
--- src/bin/pg_dump/pg_backup_archiver.c	9 Mar 2009 22:33:32 -0000
***************
*** 3467,3478 ****
  
  	/*
  	 * Close and reopen the input file so we have a private file pointer
! 	 * that doesn't stomp on anyone else's file pointer.
  	 *
! 	 * Note: on Windows, since we are using threads not processes, this
! 	 * *doesn't* close the original file pointer but just open a new one.
  	 */
! 	(AH->ReopenPtr) (AH);
  
  	/*
  	 * We need our own database connection, too
--- 3467,3486 ----
  
  	/*
  	 * Close and reopen the input file so we have a private 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, the
! 	 * reopen call *doesn't* close the original file pointer but just open 
! 	 * a new one.
  	 */
! 	if (te->section == SECTION_DATA )
! 		(AH->ReopenPtr) (AH);
! #ifndef WIN32
! 	else
! 		(AH->ClosePtr) (AH);
! #endif;
  
  	/*
  	 * We need our own database connection, too
***************
*** 3490,3495 ****
--- 3498,3505 ----
  	PQfinish(AH->connection);
  	AH->connection = NULL;
  
+ 	/* 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)
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.528
diff -c -r1.528 pg_dump.c
*** src/bin/pg_dump/pg_dump.c	4 Mar 2009 11:57:00 -0000	1.528
--- src/bin/pg_dump/pg_dump.c	9 Mar 2009 22:33:32 -0000
***************
*** 287,292 ****
--- 287,294 ----
  
  	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
  
+ 	init_dump_utils();
+ 
  	g_verbose = false;
  
  	strcpy(g_comment_start, "-- ");
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.119
diff -c -r1.119 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c	4 Mar 2009 11:57:00 -0000	1.119
--- src/bin/pg_dump/pg_dumpall.c	9 Mar 2009 22:33:32 -0000
***************
*** 136,141 ****
--- 136,143 ----
  
  	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
  
+ 	init_dump_utils();
+ 
  	progname = get_progname(argv[0]);
  
  	if (argc > 1)
Index: src/bin/pg_dump/pg_restore.c
===================================================================
RCS file: /home/cvsmirror/pg/pgsql/src/bin/pg_dump/pg_restore.c,v
retrieving revision 1.94
diff -c -r1.94 pg_restore.c
*** src/bin/pg_dump/pg_restore.c	26 Feb 2009 16:02:38 -0000	1.94
--- src/bin/pg_dump/pg_restore.c	9 Mar 2009 22:33:32 -0000
***************
*** 40,45 ****
--- 40,46 ----
   */
  
  #include "pg_backup_archiver.h"
+ #include "dumputils.h"
  
  #include <ctype.h>
  
***************
*** 125,130 ****
--- 126,133 ----
  
  	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
  
+ 	init_dump_utils();
+ 
  	opts = NewRestoreOptions();
  
  	progname = get_progname(argv[0]);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to