in spawn_restore:
child = (HANDLE) _beginthreadex(NULL, 0, (void *) parallel_restore,
args,
0, NULL);
if (child == 0)
But from my reading of the docs, _beginthreadex() returns -1 on error, not 0.
Bug, or
On Sat, Feb 26, 2011 at 16:51, Magnus Hagander mag...@hagander.net wrote:
in spawn_restore:
child = (HANDLE) _beginthreadex(NULL, 0, (void *) parallel_restore,
args,
0, NULL);
if (child == 0)
But from
Andrew Dunstan wrote:
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
There is an unfinished TODO item here: we really ought to make it work
for tar-format archives. That's probably not hugely difficult, but
I didn't look into it, and don't
Tom Lane wrote:
I wrote:
Andrew Dunstan and...@dunslane.net writes:
In my original patch, I looked at all the dependencies of a candidate
item ansd compared them with the dependencies of the running items to
see if there was a potential locking clash. However, Tom in his
admirable
I wrote:
Andrew Dunstan and...@dunslane.net writes:
In my original patch, I looked at all the dependencies of a candidate
item ansd compared them with the dependencies of the running items to
see if there was a potential locking clash. However, Tom in his
admirable reworking of my patch,
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
OK, here 'tis.
Looks fairly reasonable to me, but of course I haven't tested it.
Well, I have to do a blitz of parallel restores next week, so hopefully
that will hit any soft spots.
--Josh
--
Sent via pgsql-hackers mailing list
Josh Berkus wrote:
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
OK, here 'tis.
Looks fairly reasonable to me, but of course I haven't tested it.
Well, I have to do a blitz of parallel restores next week, so
hopefully that will hit any soft spots.
I have a known
OK, I've worked out why I am seeing deadlocks etc. from parallel restore
on FK items.
In my original patch, I looked at all the dependencies of a candidate
item ansd compared them with the dependencies of the running items to
see if there was a potential locking clash. However, Tom in his
Andrew Dunstan and...@dunslane.net writes:
OK, I've worked out why I am seeing deadlocks etc. from parallel restore
on FK items.
In my original patch, I looked at all the dependencies of a candidate
item ansd compared them with the dependencies of the running items to
see if there was a
Tom Lane wrote:
How about this: by default, fmtId uses the same logic as now (one static
PQExpBuffer). If told to by a call of init_parallel_dump_utils(), which
need only be called by pg_restore during its startup, then it switches to
using per-thread storage. init_parallel_dump_utils can
Andrew Dunstan and...@dunslane.net writes:
OK, here 'tis.
Looks fairly reasonable to me, but of course I haven't tested it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
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
Andrew Dunstan wrote:
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
Hmm, if pg_restore is the only program that's threaded, why are
Andrew Dunstan and...@dunslane.net writes:
+ void
+ init_dump_utils()
This should be
+ void
+ init_dump_utils(void)
please. We don't do KR C around here. I'd lose the added retval
variable too; that's not contributing anything.
! #endif;
Semicolon is bogus here.
Looks pretty sane
Alvaro Herrera alvhe...@commandprompt.com writes:
Hmm, if pg_restore is the only program that's threaded, why are you
calling init_dump_utils on pg_dump and pg_dumpall?
... because fmtId will crash on *any* use without that.
It makes me a bit
nervous because there are some other programs
Tom Lane wrote:
It makes me a bit
nervous because there are some other programs that are linking
dumputils.c (psql and some in src/bin/scripts/) and even calling fmtId.
Actually, why bother with init_dump_utils at all? fmtId could be made
to initialize the ID variable for itself on
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
Actually, why bother with init_dump_utils at all?
Well, the Windows reference I have suggests TlsAlloc() needs to be
called early in the initialisation process ...
How early is early? The proposed call sites for init_dump_utils
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
Actually, why bother with init_dump_utils at all?
Well, the Windows reference I have suggests TlsAlloc() needs to be
called early in the initialisation process ...
How early is early? The
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
How early is early? The proposed call sites for init_dump_utils seem
already long past the point where any libc-level infrastructure would
think it is initialization time.
Well, I think at least it needs to be done where other
I wrote:
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
There is an unfinished TODO item here: we really ought to make it work
for tar-format archives. That's probably not hugely difficult, but
I didn't look into it, and don't think we should hold up
I wrote:
Once we have that the custom format code should fail on this no matter
how the dump was made, and parallel restore should work with tar
format once we add code to it to seek for data members.
s/should fail/should not fail/
:-)
cheers
andrew
--
Sent via pgsql-hackers
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
There is an unfinished TODO item here: we really ought to make it work
for tar-format archives. That's probably not hugely difficult, but
I didn't look into it, and don't think we should hold up applying the
Peter Eisentraut pete...@gmx.net writes:
Andrew Dunstan wrote:
I didn't know such a thing even existed. What causes it to happen? I
agree it should be forbidden.
It was the only way to switch users before we had SET SESSION
AUTHORIZATION and SET ROLE and such. But the pg_restore man page
I wrote:
I'm almost done reviewing the patch, and will send along an updated
version shortly.
And here 'tis. I didn't commit because I have no way to test whether
I broke the Windows code path. Please test, and commit if OK.
There is an unfinished TODO item here: we really ought to make it
Tom Lane wrote:
I wrote:
I'm almost done reviewing the patch, and will send along an updated
version shortly.
And here 'tis.
Many many thanks. Your edits look very sensible, as always.
I didn't commit because I have no way to test whether
I broke the Windows code path. Please
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
There is an unfinished TODO item here: we really ought to make it work
for tar-format archives. That's probably not hugely difficult, but
I didn't look into it, and don't think we should hold up applying the
existing patch for it.
Tom Lane wrote:
And here 'tis. I didn't commit because I have no way to test whether
I broke the Windows code path. Please test, and commit if OK.
Tested and committed.
Thanks to the people who reviewed and tested this - it was quite a
difficult piece of work, much more difficult
Andrew Dunstan wrote:
Still, that's not a 100% solution because of the cases where we use
reconnections to change user IDs --- the required password would
(usually) vary. It might be sufficient to forbid that case with
parallel restore, though; I think it's mostly a legacy thing anyway.
I
Tom Lane wrote:
Okay, another question --- there are two places in pg_backup_custom.c
where the patch #ifdef's out hasSeek tests on WIN32. Why is that?
If checkSeek() is wrong on Windows, wouldn't it be better to fix it?
Oh, dear. That's a hangover from before that got fixed
Andrew Dunstan wrote:
Tom Lane wrote:
Okay, another question --- there are two places in pg_backup_custom.c
where the patch #ifdef's out hasSeek tests on WIN32. Why is that?
If checkSeek() is wrong on Windows, wouldn't it be better to fix it?
Oh, dear. That's a hangover from before
Andrew Dunstan and...@dunslane.net writes:
Latest patch is attached.
Starting to look at this now. One thing that is bothering me is that
if the connection parameters are such as to cause prompts for passwords,
it's going to be broken beyond usability (multiple threads all trying
to read the
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Latest patch is attached.
Starting to look at this now.
Excellent!
One thing that is bothering me is that
if the connection parameters are such as to cause prompts for passwords,
it's going to be broken beyond
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
One thing that is bothering me is that
if the connection parameters are such as to cause prompts for passwords,
it's going to be broken beyond usability (multiple threads all trying
to read the terminal at once). Is there anything we
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
One thing that is bothering me is that
if the connection parameters are such as to cause prompts for passwords,
it's going to be broken beyond usability (multiple threads all trying
to read the terminal at
Okay, another question --- there are two places in pg_backup_custom.c
where the patch #ifdef's out hasSeek tests on WIN32. Why is that?
If checkSeek() is wrong on Windows, wouldn't it be better to fix it?
regards, tom lane
--
Sent via pgsql-hackers mailing list
Latest patch is attached. Changed as discussed to issue TRUNCATE ...
ONLY when talking to servers = 8.4 instead of plain TRUNCATE.
cheers
andrew
parallel_restore_15.patch.gz
Description: GNU Zip compressed data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make
Attached is the latest version.
Changes:
. some tidy up as variously requested.
. some common code is factored out
. some descriptive comments added
. platform specific stuff (e.g. spawn, reap) is factored out
. --truncate_before_load is gone, and we now do this during parallel
restore if we
Jaime Casanova wrote:
Anyway i tried to run with
--truncate-before-load and got a message about that should be
necessary to run TRUNCATE CASCADE instead.
Actually, this raises an interesting point. It doesn't seem safe to
truncate before loading unless we have just created the table
Andrew Dunstan and...@dunslane.net writes:
Now, we could decide that we always want to do a safe truncate in a
parallel restore (i.e. if we have created the table in the same
restore), even if archive_mode is on. Then this switch would be
redundant, and we might avoid some confusion. I'm
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan and...@dunslane.net wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
hi, i was making some tests in windows...
but for some reason the pg_restore simply hangs...
i'm using:
pg_restore -f mic.backup
On Tue, Jan 6, 2009 at 4:04 PM, Jaime Casanova
jcasa...@systemguards.com.ec wrote:
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan and...@dunslane.net wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
hi, i was making some tests in windows...
Jaime Casanova wrote:
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan and...@dunslane.net wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
hi, i was making some tests in windows...
but for some reason the pg_restore simply hangs...
i'm
Andrew Dunstan and...@dunslane.net writes:
Jaime Casanova wrote:
i'm using:
pg_restore -f mic.backup -Fc -v -m5
Strange. Maybe the server log will show activity?
There's no connection info, so that should just print to stdout, and
probably there is no point in any parallelism. I'm guessing
On Tue, Jan 6, 2009 at 4:32 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Andrew Dunstan and...@dunslane.net writes:
Jaime Casanova wrote:
i'm using:
pg_restore -f mic.backup -Fc -v -m5
Strange. Maybe the server log will show activity?
There's no connection info, so that should just print to
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan and...@dunslane.net wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
mmm... seems this patch are two in one... you're adding --multi-thread
and --truncate-before-load options where the second one seems
Jaime Casanova wrote:
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan and...@dunslane.net wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
mmm... seems this patch are two in one... you're adding --multi-thread
and --truncate-before-load
On Tue, Jan 6, 2009 at 5:54 PM, Andrew Dunstan and...@dunslane.net wrote:
Well, the only reason it was needed was because you can't run a parallel
restore in a single transaction. If the whole restore is run in a single
transaction then truncate before load should be unnecessary.
doesn't
Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
I propose to commit this unless someone wants more time for reviewing.
A moment's eyeballing of the patch finds rather a lot of debugging cruft
yet (inserted printfs, #ifdef WIN32blah, etc); also I think the
addition to
On Mon, 2008-12-29 at 18:42 -0500, Andrew Dunstan wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Includes help text, docco and some extra error checking.
Very brief review.
Hopefully the --truncate-before-load option works in both parallel mode
Andrew Dunstan wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Some random comments
- please #define the return type of prestore(). Also, that it returns
in one platform and exits in another seems weird as an API. I think it
should return in
Andrew Dunstan and...@dunslane.net writes:
I propose to commit this unless someone wants more time for reviewing.
A moment's eyeballing of the patch finds rather a lot of debugging cruft
yet (inserted printfs, #ifdef WIN32blah, etc); also I think the
addition to include/port.h belongs in
I wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Includes help text, docco and some extra error checking.
I propose to commit this unless someone wants more time for reviewing.
cheers
andrew
--
Sent via pgsql-hackers mailing list
Andrew Dunstan wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Just some details, you often mix tab and spaces for indentation...
What's the standard in pgsql ?
--
Laurent COUSTET
--
Sent via pgsql-hackers mailing list
Laurent Coustet wrote:
Andrew Dunstan wrote:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Just some details, you often mix tab and spaces for indentation...
What's the standard in pgsql ?
It's tabs, see:
Attached is the latest parallel restore patch. I think this is getting
fairly close.
Includes help text, docco and some extra error checking.
cheers
andrew
parallel_restore_13.patch.gz
Description: GNU Zip compressed data
--
Sent via pgsql-hackers mailing list
On Sun, Dec 14, 2008 at 12:13 PM, Andrew Dunstan and...@dunslane.net wrote:
This version completes properly on Windows with the regression database.
actually, this one doesn't apply cleanly on head
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de
Jaime Casanova wrote:
On Sun, Dec 14, 2008 at 12:13 PM, Andrew Dunstan and...@dunslane.net wrote:
This version completes properly on Windows with the regression database.
actually, this one doesn't apply cleanly on head
I will have a new patch a day or two after Christmas,
Andrew Dunstan and...@dunslane.net wrote:
I did this, but it turned out that the problem was a logic error that I
found once I had managed to get a working debugger. However, the Windows
thread code should now be more robust, so thanks to Andrew and Magnus
for the suggestions.
Hello, I
ITAGAKI Takahiro wrote:
Andrew Dunstan and...@dunslane.net wrote:
I did this, but it turned out that the problem was a logic error that I
found once I had managed to get a working debugger. However, the Windows
thread code should now be more robust, so thanks to Andrew and Magnus
for
Andrew Chernow wrote:
If it previously worked without threads, than in theory a deep copy of
the thread_arg should fix the core dump; especially if the non-windows
fork() method works with this patch. Maybe you can get away with only
copying some of the members (trial-n-error), I don't
Andrew Chernow wrote:
Looks like the ArchiveHandle variable 'AH' and the TocEntry
'next_work_item' are not being deep copied at line 315 of your patch,
where you prepare the RestoreArgs struct for the thread. Every thread
is accessing and possibly updating the members of these structs
Parts of AH need deep cloning, notably the formatData member, which is
done in _ReopenArchive().
Is it okay to clone this from within the thread?
The reopen() appears to mess with AH-FH, which mutltiple threads are
calling fclose on. The second thread is going to fail and the first
Andrew Chernow wrote:
Parts of AH need deep cloning, notably the formatData member, which
is done in _ReopenArchive().
Is it okay to clone this from within the thread?
I don't see why not.
The reopen() appears to mess with AH-FH, which mutltiple threads are
calling fclose on. The
Andrew Dunstan wrote:
No, as this fragment shows, fclose() is NOT called on Windows.
Oooppps. I'm the village idiot today.
The program dies with a nasty dialog box when restoring a dump of the
regression database after the second COPY thread disconnects.
I'll poke around but
Parts of AH need deep cloning, notably the formatData member, which
is done in _ReopenArchive().
Is it okay to clone this from within the thread?
I don't see why not.
Because another thread may be modifying the memory you are trying to
clone. If no one modifies the formatData struct,
Andrew Chernow wrote:
Parts of AH need deep cloning, notably the formatData member, which
is done in _ReopenArchive().
Is it okay to clone this from within the thread?
I don't see why not.
Because another thread may be modifying the memory you are trying to
clone. If no one
Andrew Dunstan wrote:
Andrew Chernow wrote:
Parts of AH need deep cloning, notably the formatData member, which
is done in _ReopenArchive().
Is it okay to clone this from within the thread?
I don't see why not.
Because another thread may be modifying the memory you are trying to
Magnus Hagander [EMAIL PROTECTED] writes:
Andrew Dunstan wrote:
I'll try. It's unfortunately not as simple as it sounds, because of the
way the abstractions are arranged. I can't count the number of times I
have had to stop and try to clear my head while working on this code.
That's what
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
Andrew Dunstan wrote:
I'll try. It's unfortunately not as simple as it sounds, because of the
way the abstractions are arranged. I can't count the number of times I
have had to stop and try to clear my head while working on this code.
Magnus Hagander wrote:
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
Andrew Dunstan wrote:
I'll try. It's unfortunately not as simple as it sounds, because of the
way the abstractions are arranged. I can't count the number of times I
have had to stop and try to clear my head while
Tom Lane wrote:
I think pretty much everybody except Philip Warner has found the stuff
around the TOC data structure and the archiver API to be confusing.
I'm not immediately sure about a better design though, at least not if
you don't want to duplicate a lot of code between the plain pg_dump
Philip Warner wrote:
Tom Lane wrote:
I think pretty much everybody except Philip Warner has found the stuff
around the TOC data structure and the archiver API to be confusing.
I'm not immediately sure about a better design though, at least not if
you don't want to duplicate a lot of code
OK, after quite some trying I have hit a brick wall. I have been unable
to get parallel restore to work with Windows threading. No doubt I am
missing something, but I really don't know what. Unless someone can tell
me what I am doing wrong, I have these possibilities:
* run parallel
Andrew Dunstan wrote:
OK, after quite some trying I have hit a brick wall. I have been unable
to get parallel restore to work with Windows threading. No doubt I am
missing something, but I really don't know what. Unless someone can tell
me what I am doing wrong, I have these possibilities:
Andrew Chernow wrote:
Andrew Dunstan wrote:
OK, after quite some trying I have hit a brick wall. I have been
unable to get parallel restore to work with Windows threading. No
doubt I am missing something, but I really don't know what. Unless
someone can tell me what I am doing wrong, I
HANDLE h = (HANDLE)_beginthreadex(NULL, 0, thread_start, arg, 0, NULL);
This didn't give me any more joy, unfortunately. But you're right, I
should be using it.
Are these threads sharing memory, intentionally or by mistake?
if(h)
CloseHandle(h);
Umm, even if I wait on the handle
Andrew Chernow wrote:
HANDLE h = (HANDLE)_beginthreadex(NULL, 0, thread_start, arg, 0, NULL);
This didn't give me any more joy, unfortunately. But you're right, I
should be using it.
Are these threads sharing memory, intentionally or by mistake?
Things they write, and things they
Andrew Dunstan wrote:
Andrew Chernow wrote:
HANDLE h = (HANDLE)_beginthreadex(NULL, 0, thread_start, arg, 0, NULL);
This didn't give me any more joy, unfortunately. But you're right, I
should be using it.
Are these threads sharing memory, intentionally or by mistake?
Things they
Are these threads sharing memory, intentionally or by mistake?
Things they write, and things they read but might not be stable, are
not supposed to be shared. If they are it's a mistake.
Looks like the ArchiveHandle variable 'AH' and the TocEntry
'next_work_item' are not being deep
Attached is the latest version of my parallel restore work.
In this version pg_dump has learned how to tag each item with the
section it belongs to (data, pre-data, post-data). That removes the
necessity for hardcoding knowledge about the boundary of the data
section into the parallel
Andrew Dunstan wrote:
Overall result: baseline: 4h32m parallel: 0h 54m.
Awesome!!
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list
I have just completed a test of the patch I posted a few days ago.
The test is a 2Gb dump file that restores to a 22Gb database. The
database is very complex, with some 28,000 objects.
The baseline test was run in a single transaction:
pg_restore --use-list tlist -1 -d mdata
82 matches
Mail list logo
