Re: [HACKERS] patch for parallel pg_dump

2012-04-04 Thread Joachim Wieland
On Tue, Apr 3, 2012 at 11:04 AM, Robert Haas robertmh...@gmail.com wrote: OK, but it seems like a pretty fragile assumption that none of the workers will ever manage to emit any other error messages.  We don't rely on this kind of assumption in the backend, which is a lot better-structured and

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Sun, Apr 1, 2012 at 12:35 PM, Joachim Wieland j...@mcknight.de wrote: On Wed, Mar 28, 2012 at 2:20 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: My main comment about the current patch is that it looks like it's touching pg_restore parallel code by moving some stuff into parallel.c.

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Joachim Wieland
On Tue, Apr 3, 2012 at 9:34 AM, Robert Haas robertmh...@gmail.com wrote: On Sun, Apr 1, 2012 at 12:35 PM, Joachim Wieland j...@mcknight.de wrote: Unfortunately this is not really the case. What is being moved out of pg_backup_archiver.c and into parallel.c is either the shutdown logic that has

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Alvaro Herrera
Excerpts from Joachim Wieland's message of mar abr 03 11:40:31 -0300 2012: On Tue, Apr 3, 2012 at 9:34 AM, Robert Haas robertmh...@gmail.com wrote: Hmm.  It looks to me like the part-two patch still contains a bunch of code rearrangement.  For example, the current code for

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Tue, Apr 3, 2012 at 10:40 AM, Joachim Wieland j...@mcknight.de wrote: On a similar note, what's the point of changing struct Archive to have int numWorkers instead of int number_of_jobs, and furthermore shuffling the declaration around to a different part of the struct? number_of_jobs was

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 10:40 AM, Joachim Wieland j...@mcknight.de wrote: I completely agree. Assertions helped a lot dealing with concurrent code. How do you want to tackle this for now? Want me to create a separate header pg_assert.h as part of my

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Tue, Apr 3, 2012 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 10:40 AM, Joachim Wieland j...@mcknight.de wrote: I completely agree. Assertions helped a lot dealing with concurrent code. How do you want to tackle this for

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Alvaro Herrera
Excerpts from Tom Lane's message of mar abr 03 12:38:20 -0300 2012: Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 10:40 AM, Joachim Wieland j...@mcknight.de wrote: I completely agree. Assertions helped a lot dealing with concurrent code. How do you want to tackle this

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote: Possibly we could move assert.c into src/port/ and make it part of libpgport? The trouble is that it calls write_stderr(), which has a non-trivial implementation on Windows that I

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Tue, Apr 3, 2012 at 11:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote: Possibly we could move assert.c into src/port/ and make it part of libpgport? The trouble is that it calls

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes: That only leaves assert_enabled to be handled. In the backend it lives in guc.c; what to do about frontend code? There's no mechanism for turning such a switch on or off in most frontend code anyway. I'd think it could just be assumed to be on

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 11:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, if we don't have a solution to that problem then it's premature to propose making Assert available to frontend code.  So my opinion is that that idea is too half-baked to be

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Tue, Apr 3, 2012 at 12:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 11:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Well, if we don't have a solution to that problem then it's premature to propose making Assert available to frontend

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 12:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: No, the reason for write_stderr() is that fprintf(stderr) is unreliable on Windows.  If memory serves, it can actually crash in some situations. Dude, we're already doing

Re: [HACKERS] patch for parallel pg_dump

2012-04-03 Thread Robert Haas
On Tue, Apr 3, 2012 at 12:37 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Apr 3, 2012 at 12:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: No, the reason for write_stderr() is that fprintf(stderr) is unreliable on Windows.  If memory serves, it can

Re: [HACKERS] patch for parallel pg_dump

2012-03-29 Thread Robert Haas
On Wed, Mar 28, 2012 at 9:54 PM, Joachim Wieland j...@mcknight.de wrote: On Wed, Mar 28, 2012 at 1:46 PM, Robert Haas robertmh...@gmail.com wrote: I'm wondering if we really need this much complexity around shutting down workers.  I'm not sure I understand why we need both a hard and a soft

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Robert Haas
On Sun, Mar 25, 2012 at 10:50 PM, Joachim Wieland j...@mcknight.de wrote: On Fri, Mar 23, 2012 at 11:11 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Are you going to provide a rebased version? Rebased version attached, this patch also includes Robert's earlier suggestions. I keep

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Alvaro Herrera
Excerpts from Robert Haas's message of mié mar 28 14:46:30 -0300 2012: I keep hoping someone who knows Windows is going to take a look at this, but so far no luck. It could also really use some attention from someone who has an actual really big database handy, to see how successful it is

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Robert Haas
On Wed, Mar 28, 2012 at 2:20 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mié mar 28 14:46:30 -0300 2012: I keep hoping someone who knows Windows is going to take a look at this, but so far no luck.  It could also really use some attention from

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Andrew Dunstan
On 03/28/2012 02:27 PM, Robert Haas wrote: On Wed, Mar 28, 2012 at 2:20 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mié mar 28 14:46:30 -0300 2012: I keep hoping someone who knows Windows is going to take a look at this, but so far no luck.

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Andrew Dunstan
On 03/28/2012 03:17 PM, Andrew Dunstan wrote: On 03/28/2012 02:27 PM, Robert Haas wrote: On Wed, Mar 28, 2012 at 2:20 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mié mar 28 14:46:30 -0300 2012: I keep hoping someone who knows Windows is

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Joachim Wieland
On Wed, Mar 28, 2012 at 5:19 PM, Andrew Dunstan and...@dunslane.net wrote: First hurdle: It doesn't build under Windows/mingw-w64:   parallel.c:40:12: error: static declaration of 'pgpipe' follows   non-static declaration Strange, I'm not seeing this but I'm building with VC2005. What happens

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Andrew Dunstan
On 03/28/2012 08:28 PM, Joachim Wieland wrote: On Wed, Mar 28, 2012 at 5:19 PM, Andrew Dunstanand...@dunslane.net wrote: First hurdle: It doesn't build under Windows/mingw-w64: parallel.c:40:12: error: static declaration of 'pgpipe' follows non-static declaration Strange, I'm not

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Joachim Wieland
On Thu, Mar 29, 2012 at 2:46 AM, Andrew Dunstan and...@dunslane.net wrote: But your patch hasn't got rid of them, and so it's declared twice. There is no pgpipe.h, BTW, it's declared in port.h. If VC2005 doesn't complain about the double declaration then that's a bug in the compiler, IMNSHO.

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Joachim Wieland
On Wed, Mar 28, 2012 at 1:46 PM, Robert Haas robertmh...@gmail.com wrote: I'm wondering if we really need this much complexity around shutting down workers.  I'm not sure I understand why we need both a hard and a soft method of shutting them down.  At least on non-Windows systems, it seems

Re: [HACKERS] patch for parallel pg_dump

2012-03-28 Thread Andrew Dunstan
On 03/28/2012 09:12 PM, Joachim Wieland wrote: On Thu, Mar 29, 2012 at 2:46 AM, Andrew Dunstanand...@dunslane.net wrote: But your patch hasn't got rid of them, and so it's declared twice. There is no pgpipe.h, BTW, it's declared in port.h. If VC2005 doesn't complain about the double

Re: [HACKERS] patch for parallel pg_dump

2012-03-25 Thread Joachim Wieland
On Fri, Mar 23, 2012 at 11:11 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Are you going to provide a rebased version? Rebased version attached, this patch also includes Robert's earlier suggestions. parallel_pg_dump_5.diff.gz Description: GNU Zip compressed data -- Sent via

Re: [HACKERS] patch for parallel pg_dump

2012-03-23 Thread Alvaro Herrera
Are you going to provide a rebased version? -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to

Re: [HACKERS] patch for parallel pg_dump

2012-03-23 Thread Joachim Wieland
On Fri, Mar 23, 2012 at 11:11 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Are you going to provide a rebased version? Yes, working on that. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] patch for parallel pg_dump

2012-03-20 Thread Alvaro Herrera
Excerpts from Joachim Wieland's message of mar mar 20 08:26:52 -0300 2012: On Tue, Mar 20, 2012 at 12:03 AM, Erik Rijkers e...@xs4all.nl wrote: In my hands, the patch complains: Thanks, updated patch attached. Applied, with some minor tweaks, thanks. I didn't try the WIN32 compile. I

Re: [HACKERS] patch for parallel pg_dump

2012-03-19 Thread Alvaro Herrera
Excerpts from Joachim Wieland's message of lun mar 19 00:31:47 -0300 2012: On Wed, Mar 14, 2012 at 2:02 PM, Robert Haas robertmh...@gmail.com wrote: I think we should somehow unify both functions, the code is not very consistent in this respect, it also calls exit_horribly() when it has AH

Re: [HACKERS] patch for parallel pg_dump

2012-03-19 Thread Erik Rijkers
On Tue, March 20, 2012 04:04, Joachim Wieland wrote: On Mon, Mar 19, 2012 at 9:14 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Sounds good to me in general ... my only gripe is this: I wonder if it would be better to have a central routine that knows about both

Re: [HACKERS] patch for parallel pg_dump

2012-03-19 Thread Erik Rijkers
[pg_dump_die_horribly.2.diff ] In my hands, the patch complains: In file included from gram.y:13255:0: scan.c: In function ‘yy_try_NUL_trans’: scan.c:16243:23: warning: unused variable ‘yyg’ [-Wunused-variable] pg_backup_archiver.c:3320:1: error: static declaration of

Re: [HACKERS] patch for parallel pg_dump

2012-03-17 Thread Joachim Wieland
On Fri, Mar 16, 2012 at 12:06 AM, Robert Haas robertmh...@gmail.com wrote: Good. The only exit handler I've seen so far is pgdump_cleanup_at_exit. If there's no other one, is it okay to remove all of this stacking functionality (see on_exit_nicely_index / MAX_ON_EXIT_NICELY) from dumputils.c

Re: [HACKERS] patch for parallel pg_dump

2012-03-15 Thread Robert Haas
On Thu, Mar 15, 2012 at 12:56 AM, Joachim Wieland j...@mcknight.de wrote: On Wed, Mar 14, 2012 at 2:02 PM, Robert Haas robertmh...@gmail.com wrote: I think we should get rid of die_horribly(), and instead have arrange to always clean up AH via an on_exit_nicely hook. Good. The only exit

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Robert Haas
On Wed, Mar 14, 2012 at 12:34 AM, Joachim Wieland j...@mcknight.de wrote: If a child terminates without leaving a message, the master will still detect it and just say a worker process died unexpectedly (this part was actually broken, but now it's fixed :-) ) All that may be true, but I still

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Andrew Dunstan
On 03/13/2012 02:10 PM, Andrew Dunstan wrote: On 03/13/2012 01:53 PM, Robert Haas wrote: I tried this actually (patch attached) but then I wanted to test it and couldn't find anything that used pgpipe() on Windows. pg_basebackup/pg_basebackup.c is using it but it's in an #ifndef WIN32 and

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Alvaro Herrera
Excerpts from Andrew Dunstan's message of mié mar 14 17:39:59 -0300 2012: pgpipe used to be used in pgstat.c, but that's no longer true in any live branch, so it's probably long dead. I'd be inclined to rip it out if possible rather than expand its use. our pgpipe() function is interesting

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Robert Haas
On Wed, Mar 14, 2012 at 4:39 PM, Andrew Dunstan aduns...@postgresql.org wrote: I've just started looking at the patch, and I'm curious to know why it didn't follow the pattern of parallel pg_restore which created a new worker for each table rather than passing messages to looping worker threads

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Joachim Wieland
On Wed, Mar 14, 2012 at 2:02 PM, Robert Haas robertmh...@gmail.com wrote: I think we should get rid of die_horribly(), and instead have arrange to always clean up AH via an on_exit_nicely hook. Good. The only exit handler I've seen so far is pgdump_cleanup_at_exit. If there's no other one, is

Re: [HACKERS] patch for parallel pg_dump

2012-03-14 Thread Joachim Wieland
On Wed, Mar 14, 2012 at 4:39 PM, Andrew Dunstan aduns...@postgresql.org wrote: I've just started looking at the patch, and I'm curious to know why it didn't follow the pattern of parallel pg_restore which created a new worker for each table rather than passing messages to looping worker threads

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Tom Lane
Joachim Wieland j...@mcknight.de writes: On Sat, Mar 10, 2012 at 9:51 AM, Robert Haas robertmh...@gmail.com wrote: -const char *owner, bool withOids, +const char *owner, +unsigned long int relpages, bool withOids, The

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote: (I'm also unconvinced that sorting by relation size is a good idea anyway. Anything that makes the dump order less predictable gets push-back, IME.) Given that people often use diff on files from pg_dump, unpredictable ordering can be a bad thing. On the

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Andres Freund
On Tuesday, March 13, 2012 02:48:11 PM Tom Lane wrote: (I'm also unconvinced that sorting by relation size is a good idea anyway. Anything that makes the dump order less predictable gets push-back, IME.) Why? Especially in the directory format - which is a prerequisite for parallel dump if I

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Robert Haas
On Mon, Mar 12, 2012 at 11:35 PM, Joachim Wieland j...@mcknight.de wrote: How do you mean it's unused? pg_dump_sort.c uses relpages to dump the largest tables first. What you don't want to see in a parallel dump is a worker starting to dump a large table while everybody else is already idle...

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Andrew Dunstan
On 03/13/2012 01:53 PM, Robert Haas wrote: I tried this actually (patch attached) but then I wanted to test it and couldn't find anything that used pgpipe() on Windows. pg_basebackup/pg_basebackup.c is using it but it's in an #ifndef WIN32 and the same is true for postmaster/syslogger.c. Am

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Robert Haas
On Tue, Mar 13, 2012 at 9:59 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Tom Lane t...@sss.pgh.pa.us wrote: (I'm also unconvinced that sorting by relation size is a good idea anyway.  Anything that makes the dump order less predictable gets push-back, IME.) Given that people often

Re: [HACKERS] patch for parallel pg_dump

2012-03-13 Thread Joachim Wieland
On Tue, Mar 13, 2012 at 1:53 PM, Robert Haas robertmh...@gmail.com wrote: What I mean is that the function ArchiveEntry() is defined in pg_backup_archiver.c, and it takes an argument called relpages, and the string relpages does not appear anywhere else in that file. Uhm, that's kinda

Re: [HACKERS] patch for parallel pg_dump

2012-03-10 Thread Robert Haas
On Thu, Feb 23, 2012 at 11:37 PM, Joachim Wieland j...@mcknight.de wrote: On Thu, Feb 16, 2012 at 1:29 PM, Robert Haas robertmh...@gmail.com wrote: Can you provide an updated patch? Robert, updated patch is attached. Well, I was hoping someone else would do some work on this, but here we are.

Re: [HACKERS] patch for parallel pg_dump

2012-02-23 Thread Joachim Wieland
On Thu, Feb 16, 2012 at 1:29 PM, Robert Haas robertmh...@gmail.com wrote: Can you provide an updated patch? Robert, updated patch is attached. parallel_pg_dump_2.diff.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes

Re: [HACKERS] patch for parallel pg_dump

2012-02-16 Thread Robert Haas
On Wed, Feb 8, 2012 at 7:56 PM, Joachim Wieland j...@mcknight.de wrote: So we at least need to press on far enough to get to that point. Sure, let me know if I can help you with something. Alright. I think (hope) that I've pushed this far enough to serve the needs of parallel pg_dump. The

Re: [HACKERS] patch for parallel pg_dump

2012-02-08 Thread Robert Haas
On Tue, Feb 7, 2012 at 10:21 PM, Joachim Wieland j...@mcknight.de wrote: On Tue, Feb 7, 2012 at 4:59 PM, Robert Haas robertmh...@gmail.com wrote: It turns out that (as you anticipated) there are some problems with my previous proposal. I assume you're talking to Tom, as my powers of

Re: [HACKERS] patch for parallel pg_dump

2012-02-08 Thread Joachim Wieland
On Wed, Feb 8, 2012 at 1:21 PM, Robert Haas robertmh...@gmail.com wrote: In my patch I dealt with exactly the same problem for the error handler by creating a separate function that has a static variable (a pointer to the ParallelState). The value is set and retrieved through the same

Re: [HACKERS] patch for parallel pg_dump

2012-02-07 Thread Robert Haas
On Fri, Feb 3, 2012 at 10:43 AM, Joachim Wieland j...@mcknight.de wrote: On Fri, Feb 3, 2012 at 7:52 AM, Robert Haas robertmh...@gmail.com wrote: If you're OK with that much I'll go do it. Sure, go ahead! It turns out that (as you anticipated) there are some problems with my previous

Re: [HACKERS] patch for parallel pg_dump

2012-02-07 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: most places that issue queries can simply use those routines without needing to peek under the hood into the ArchiveHandle. This is not quite enough to get rid of g_conn, but it's close: the major stumbling block at this point is probably

Re: [HACKERS] patch for parallel pg_dump

2012-02-07 Thread Joachim Wieland
On Tue, Feb 7, 2012 at 4:59 PM, Robert Haas robertmh...@gmail.com wrote: It turns out that (as you anticipated) there are some problems with my previous proposal. I assume you're talking to Tom, as my powers of anticipation are actually quite limited... :-) This is not quite enough to get

Re: [HACKERS] patch for parallel pg_dump

2012-02-03 Thread Robert Haas
On Thu, Feb 2, 2012 at 8:31 PM, Joachim Wieland j...@mcknight.de wrote: On Wed, Feb 1, 2012 at 12:24 PM, Robert Haas robertmh...@gmail.com wrote: I think we're more-or-less proposing to rename Archive to Connection, aren't we? And then ArchiveHandle can store all the things that aren't

Re: [HACKERS] patch for parallel pg_dump

2012-02-03 Thread Joachim Wieland
On Fri, Feb 3, 2012 at 7:52 AM, Robert Haas robertmh...@gmail.com wrote: If you're OK with that much I'll go do it. Sure, go ahead! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] patch for parallel pg_dump

2012-02-02 Thread Joachim Wieland
On Wed, Feb 1, 2012 at 12:24 PM, Robert Haas robertmh...@gmail.com wrote: I think we're more-or-less proposing to rename Archive to Connection, aren't we? And then ArchiveHandle can store all the things that aren't related to a specific connection. How about something like that: (Hopefully

Re: [HACKERS] patch for parallel pg_dump

2012-02-01 Thread Robert Haas
On Tue, Jan 31, 2012 at 4:46 PM, Joachim Wieland j...@mcknight.de wrote: On Tue, Jan 31, 2012 at 9:05 AM, Robert Haas robertmh...@gmail.com wrote: And just for added fun and excitement, they all have inconsistent naming conventions and inadequate documentation. I think if we need more

Re: [HACKERS] patch for parallel pg_dump

2012-01-31 Thread Robert Haas
On Tue, Jan 31, 2012 at 12:55 AM, Joachim Wieland j...@mcknight.de wrote: On Mon, Jan 30, 2012 at 12:20 PM, Robert Haas robertmh...@gmail.com wrote: But the immediate problem is that pg_dump.c is heavily reliant on global variables, which isn't going to fly if we want this code to use threads

Re: [HACKERS] patch for parallel pg_dump

2012-01-31 Thread Joachim Wieland
On Tue, Jan 31, 2012 at 9:05 AM, Robert Haas robertmh...@gmail.com wrote: And just for added fun and excitement, they all have inconsistent naming conventions and inadequate documentation. I think if we need more refactoring in order to support multiple database connections, we should go do

Re: [HACKERS] patch for parallel pg_dump

2012-01-30 Thread Robert Haas
On Sun, Jan 29, 2012 at 12:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: Joachim Wieland j...@mcknight.de writes: I know that you took back some of your comments, but I'm with you here. Archive is allocated as an ArchiveHandle and then casted back to Archive*, so you always know that an Archive is

Re: [HACKERS] patch for parallel pg_dump

2012-01-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: But the immediate problem is that pg_dump.c is heavily reliant on global variables, which isn't going to fly if we want this code to use threads on Windows (or anywhere else). It's also bad style. So I suggest that we refactor pg_dump.c to get rid of

Re: [HACKERS] patch for parallel pg_dump

2012-01-30 Thread Joachim Wieland
On Mon, Jan 30, 2012 at 12:20 PM, Robert Haas robertmh...@gmail.com wrote: But the immediate problem is that pg_dump.c is heavily reliant on global variables, which isn't going to fly if we want this code to use threads on Windows (or anywhere else).  It's also bad style. Technically, since

Re: [HACKERS] patch for parallel pg_dump

2012-01-29 Thread Joachim Wieland
On Fri, Jan 27, 2012 at 4:57 PM, Robert Haas robertmh...@gmail.com wrote: But even if you do know that subclassing is intended, that doesn't prove that the particular Archive object is always going to be an ArchiveHandle under the hood.  If it is, why not just pass it as an ArchiveHandle to

Re: [HACKERS] patch for parallel pg_dump

2012-01-29 Thread Tom Lane
Joachim Wieland j...@mcknight.de writes: I know that you took back some of your comments, but I'm with you here. Archive is allocated as an ArchiveHandle and then casted back to Archive*, so you always know that an Archive is an ArchiveHandle. I'm all for getting rid of Archive and just using

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Robert Haas
On Sun, Jan 15, 2012 at 1:01 PM, Joachim Wieland j...@mcknight.de wrote: So this is the parallel pg_dump patch, generalizing the existing parallel restore and allowing parallel dumps for the directory archive format, the patch works on Windows and Unix. This patch introduces a large amount of

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Robert Haas
On Fri, Jan 27, 2012 at 10:57 AM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jan 15, 2012 at 1:01 PM, Joachim Wieland j...@mcknight.de wrote: So this is the parallel pg_dump patch, generalizing the existing parallel restore and allowing parallel dumps for the directory archive format,

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Robert Haas
On Fri, Jan 27, 2012 at 10:58 AM, Robert Haas robertmh...@gmail.com wrote: It's not clear to me why fmtQualifiedId needs to move to dumputils.c. The way you have it, fmtQualifiedId() is now with fmtId(), but no longer with fmtCopyColumnList(), the only other similarly named function in that

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Robert Haas
On Sun, Jan 15, 2012 at 1:01 PM, Joachim Wieland j...@mcknight.de wrote: So this is the parallel pg_dump patch, generalizing the existing parallel restore and allowing parallel dumps for the directory archive format, the patch works on Windows and Unix. It seems a little unfortunate that we

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Heikki Linnakangas
On 27.01.2012 18:46, Robert Haas wrote: On Sun, Jan 15, 2012 at 1:01 PM, Joachim Wielandj...@mcknight.de wrote: In parallel restore, the master closes its own connection to the database before forking of worker processes, just as it does now. In parallel dump however, we need to hold the

Re: [HACKERS] patch for parallel pg_dump

2012-01-27 Thread Robert Haas
On Fri, Jan 27, 2012 at 11:53 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: If the master process keeps the locks it acquires in the beginning, you could fall back to dumping those tables where the child lock fails using the master connection. Hmm, that's a thought.