On Sat, Feb 26, 2011 at 16:51, Magnus Hagander wrote:
> in spawn_restore:
>
>
> child = (HANDLE) _beginthreadex(NULL, 0, (void *) parallel_restore,
> args,
> 0, NULL);
> if (child == 0)
>
>
> But from my reading
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 a
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
> > Andrew Dunstan 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
Tom Lane wrote:
I wrote:
Andrew Dunstan 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 patc
I wrote:
> Andrew Dunstan 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, restricted t
Andrew Dunstan 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 potential lockin
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
a
Josh Berkus wrote:
Tom Lane wrote:
Andrew Dunstan 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 outstanding bug to do with de
Tom Lane wrote:
Andrew Dunstan 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 (pgsql-hackers@post
Andrew Dunstan 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:
http://www.postgresql.org/mailpref/pgs
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 be
Andrew Dunstan 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 threads won't
Tom Lane wrote:
Andrew Dunstan 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
Andrew Dunstan 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 seem
already long
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 firs
Alvaro Herrera 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 that are linking
> dumputi
Andrew Dunstan writes:
> + void
> + init_dump_utils()
This should be
> + void
> + init_dump_utils(void)
please. We don't do K&R C around here. I'd lose the added retval
variable too; that's not contributing anything.
> ! #endif;
Semicolon is bogus here.
Looks pretty sane otherwise.
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
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
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 mailing
I wrote:
Tom Lane wrote:
Andrew Dunstan 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 p
Tom Lane wrote:
Andrew Dunstan 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 i
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 than
Andrew Dunstan 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.
> Right. Wer
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
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
Peter Eisentraut 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 says
> t
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 didn
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 tha
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 ear
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 (pgsql-ha
Tom Lane wrote:
Andrew Dunstan 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 anythi
Andrew Dunstan 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 can do abo
Tom Lane wrote:
Andrew Dunstan 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 usability (multiple threads
Andrew Dunstan 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 terminal at once). I
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 cr
Andrew Dunstan 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 inclined to do tha
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 ear
On Tue, Jan 6, 2009 at 5:54 PM, Andrew Dunstan 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 understand you. An
Jaime Casanova wrote:
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan 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 seco
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan 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 to be an
optimiz
On Tue, Jan 6, 2009 at 4:32 PM, Tom Lane wrote:
> Andrew Dunstan 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
Andrew Dunstan 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 the -m
switch
Jaime Casanova wrote:
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan 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
On Tue, Jan 6, 2009 at 4:04 PM, Jaime Casanova
wrote:
> On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan wrote:
>>
>> Attached is the latest parallel restore patch. I think this is getting
>> fairly close.
>>
>
> hi, i was making some tests in windows...
>
anyway, when i try it, it prints on the
On Mon, Dec 29, 2008 at 6:42 PM, Andrew Dunstan 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 -Fc -v -m5
there
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 b
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 m
Tom Lane wrote:
Andrew Dunstan 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
Andrew Dunstan 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 port/win32.h instead.
M
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 (pgsql-h
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:
http://www.postgresql.org/docs/8.3/s
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 (pgsql-hackers@postgresql
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 (pgsql-hackers@postgresql.o
Jaime Casanova wrote:
On Sun, Dec 14, 2008 at 12:13 PM, Andrew Dunstan 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, which I hope will
be v
On Sun, Dec 14, 2008 at 12:13 PM, Andrew Dunstan 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 sistemas
Guayaquil
ITAGAKI Takahiro wrote:
Andrew Dunstan 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.
Andrew Dunstan 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 tested parallel r
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 thi
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
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_
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 whil
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 o
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
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 b
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 modifie
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 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 apparently
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
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
fclo
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 that
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 copie
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 wr
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 re
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 usi
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 hav
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:
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 steps
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 restore
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 (pgsql-hackers@postgr
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 ../ned-int.pz
82 matches
Mail list logo
