Обсуждение: WIP parallel restore patch
Attached is my latest parallel restore patch. I think it's functionally complete for Unix. Many bugs have been fixed since the last patch, and the hardcoded limitation to two table dependencies is removed. It seems fairly robust in my recent testing. Remaining to be done: . code cleanup . better error checking in a few places . final decision re command line option names/defaults . documentation . Windows support. cheers andrew
Вложения
Andrew Dunstan wrote: > > Attached is my latest parallel restore patch. I think it's functionally > complete for Unix. > > Many bugs have been fixed since the last patch, and the hardcoded > limitation to two table dependencies is removed. It seems fairly robust > in my recent testing. this version seems to be working much better on my test setup. This patch results in a fairly nice improvment from ~170min (-m 1 --truncate-before-load) to ~39min (-m 16 --truncate-before-load) on my 8 core test box using a 70GB (uncompressed) dump consisting of 709 tables. Stefan
Stefan Kaltenbrunner wrote: > Andrew Dunstan wrote: >> >> Attached is my latest parallel restore patch. I think it's >> functionally complete for Unix. >> >> Many bugs have been fixed since the last patch, and the hardcoded >> limitation to two table dependencies is removed. It seems fairly >> robust in my recent testing. > > this version seems to be working much better on my test setup. > This patch results in a fairly nice improvment from ~170min (-m 1 > --truncate-before-load) to ~39min (-m 16 --truncate-before-load) on my > 8 core test box using a 70GB (uncompressed) dump consisting of 709 > tables. > > > Great. Thanks for the report. cheers andrew
Andrew Dunstan wrote: > > Attached is my latest parallel restore patch. I think it's functionally > complete for Unix. > > Many bugs have been fixed since the last patch, and the hardcoded > limitation to two table dependencies is removed. It seems fairly robust > in my recent testing. > > Remaining to be done: > > . code cleanup > . better error checking in a few places > . final decision re command line option names/defaults > . documentation > . Windows support. I've looked around this a bit, and it's fairly clear where the issue comes in with Windows - we get heap corruption. Most likely because we have multiple threads working on the same data somewhere. I notice for example that we're doing a shallow copy of the ArchiveHandle with a simple memcpy() for each thread. But that struct contains a number of things like file descriptors and pointers. Have you verified for each and every one of those that it actually doesn't get modified anywhere? If not, a deep copy may be needed to make sure of that. Other than that, are there any global variables that may be addressed from more than one worker? If so they need to be marked as TLS. And yes, I got hit by the lack of error checking a couple of times during my testing - it would probably be a good idea to add that as soon as possible, it helps a lot with the debugging. If I run it with just a single thread, it also crashes in PQfinish() called from die_horribly(), when trying to free conn->pgpass, which has a value (non-NULL) but is not a valid pointer. This crash happens in the worker thread, after it has logged that "fseek is required" - that's an indicator something being passed down to the thread is either wrong or being scribbled upon after the fact. I didn't dig into these questions specifically - since you have already been reading up on this code to do the patch you can probably reach the answer to them much quicker :-) So I'll stick to the questions. //Magnus
Magnus Hagander wrote: > Andrew Dunstan wrote: > >> Attached is my latest parallel restore patch. I think it's functionally >> complete for Unix. >> >> Many bugs have been fixed since the last patch, and the hardcoded >> limitation to two table dependencies is removed. It seems fairly robust >> in my recent testing. >> >> Remaining to be done: >> >> . code cleanup >> . better error checking in a few places >> . final decision re command line option names/defaults >> . documentation >> . Windows support. >> > > I've looked around this a bit, and it's fairly clear where the issue > comes in with Windows - we get heap corruption. Most likely because we > have multiple threads working on the same data somewhere. > > I notice for example that we're doing a shallow copy of the > ArchiveHandle with a simple memcpy() for each thread. But that struct > contains a number of things like file descriptors and pointers. Have you > verified for each and every one of those that it actually doesn't get > modified anywhere? If not, a deep copy may be needed to make sure of that. > > Other than that, are there any global variables that may be addressed > from more than one worker? If so they need to be marked as TLS. > > And yes, I got hit by the lack of error checking a couple of times > during my testing - it would probably be a good idea to add that as soon > as possible, it helps a lot with the debugging. > > If I run it with just a single thread, it also crashes in PQfinish() > called from die_horribly(), when trying to free conn->pgpass, which has > a value (non-NULL) but is not a valid pointer. This crash happens in the > worker thread, after it has logged that "fseek is required" - that's an > indicator something being passed down to the thread is either wrong or > being scribbled upon after the fact. > > I didn't dig into these questions specifically - since you have already > been reading up on this code to do the patch you can probably reach the > answer to them much quicker :-) So I'll stick to the questions. > > > OK, Thanks, this will help. I thought I had caught the ArchiveHandle things, but there might be one or two I missed. I'll try to have a new version in a few days. cheers andrew
Okay, I have had a chance to run some timing benchmarks. Here are my results for the parallel pg_restore patch: Ken -------------------------------------------------- Server settings: max_connections = 100 # (change requires restart) shared_buffers = 256MB # min 128kB work_mem = 128MB # min 64kB maintenance_work_mem = 256MB # min 1MB fsync = on # turns forced synchronization on or off synchronous_commit = off # immediate fsync at commit full_page_writes = on # recover from partial page writes checkpoint_segments = 10 # in logfile segments, min 1, 16MB each autovacuum = on # Enable autovacuum subprocess? 'on' The total final database size is 6.5GB. Here are the timings for the different run parallelism from 1 to 8 on a 4-core AMD box: -bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz ... real 19m3.175s user 1m2.968s sys 0m8.202s improvement - 0% -bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest /tmp/rtout.pz ... real 12m55.680s user 1m12.440s sys 0m8.343s improvement - 32% -bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest /tmp/rtout.pz ... real 9m45.056s user 1m1.892s sys 0m8.980s improvement - 49% The system only has 4 cores, but here are the results with "-m 8": -bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest /tmp/rtout.pz ... real 8m15.320s user 0m55.206s sys 0m8.678s improvement - 53%
Kenneth Marshall wrote: > Okay, I have had a chance to run some timing benchmarks. > Here are my results for the parallel pg_restore patch: > > Ken > -------------------------------------------------- > Server settings: > > max_connections = 100 # (change requires restart) > shared_buffers = 256MB # min 128kB > work_mem = 128MB # min 64kB > maintenance_work_mem = 256MB # min 1MB > > fsync = on # turns forced synchronization on or off > > synchronous_commit = off # immediate fsync at commit > > full_page_writes = on # recover from partial page writes > checkpoint_segments = 10 # in logfile segments, min 1, 16MB each > autovacuum = on # Enable autovacuum subprocess? 'on' > > The total final database size is 6.5GB. Here are the timings for > the different run parallelism from 1 to 8 on a 4-core AMD box: > > -bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz > ... > > real 19m3.175s > user 1m2.968s > sys 0m8.202s > > improvement - 0% > > -bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest /tmp/rtout.pz > ... > real 12m55.680s > user 1m12.440s > sys 0m8.343s > > improvement - 32% > > -bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest /tmp/rtout.pz > ... > real 9m45.056s > user 1m1.892s > sys 0m8.980s > > improvement - 49% > > The system only has 4 cores, but here are the results with "-m 8": > > -bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest /tmp/rtout.pz > ... > real 8m15.320s > user 0m55.206s > sys 0m8.678s > > improvement - 53% > > > Interesting. Can you try with two changes? Turn fsync off, and use the --truncate-before-load switch. In general, though, this is fairly much in line with other experience, i.e. we can get up to about n/2 times speedup with n cores. thanks andrew
On Thu, Nov 20, 2008 at 02:26:14PM -0500, Andrew Dunstan wrote: > > > Kenneth Marshall wrote: >> Okay, I have had a chance to run some timing benchmarks. >> Here are my results for the parallel pg_restore patch: >> >> Ken >> -------------------------------------------------- >> Server settings: >> >> max_connections = 100 # (change requires restart) >> shared_buffers = 256MB # min 128kB >> work_mem = 128MB # min 64kB >> maintenance_work_mem = 256MB # min 1MB >> >> fsync = on # turns forced synchronization on or off >> >> synchronous_commit = off # immediate fsync at commit >> >> full_page_writes = on # recover from partial page writes >> checkpoint_segments = 10 # in logfile segments, min 1, 16MB each >> autovacuum = on # Enable autovacuum subprocess? 'on' >> >> The total final database size is 6.5GB. Here are the timings for >> the different run parallelism from 1 to 8 on a 4-core AMD box: >> >> -bash-3.00$ time pg_restore -U postgres -p 5435 -d rttest /tmp/rtout.pz >> ... >> >> real 19m3.175s >> user 1m2.968s >> sys 0m8.202s >> >> improvement - 0% >> >> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 -d rttest >> /tmp/rtout.pz >> ... >> real 12m55.680s >> user 1m12.440s >> sys 0m8.343s >> >> improvement - 32% >> >> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 -d rttest >> /tmp/rtout.pz >> ... >> real 9m45.056s >> user 1m1.892s >> sys 0m8.980s >> >> improvement - 49% >> >> The system only has 4 cores, but here are the results with "-m 8": >> >> -bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 -d rttest >> /tmp/rtout.pz >> ... >> real 8m15.320s >> user 0m55.206s >> sys 0m8.678s >> >> improvement - 53% >> >> >> > > Interesting. > > Can you try with two changes? Turn fsync off, and use the > --truncate-before-load switch. > > In general, though, this is fairly much in line with other experience, i.e. > we can get up to about n/2 times speedup with n cores. > > thanks > > andrew > Okay, here is the same test run with: Cheers, Ken -------------------------------------------------------------------- fsync = off --truncate-before-load -bash-3.00$ time pg_restore -U postgres -p 5435 --truncate-before-load -d rttest/tmp/rtout.pz ... real 16m25.031s user 1m3.707s sys 0m8.776s improvement - 0% -bash-3.00$ time pg_restore -U postgres -p 5435 -m 2 --truncate-before-load -d r ttest /tmp/rtout.pz ... real 10m26.730s user 0m48.782s sys 0m7.214s improvement - 36% -bash-3.00$ time pg_restore -U postgres -p 5435 -m 4 --truncate-before-load -d r ttest /tmp/rtout.pz ... real 8m5.061s user 0m48.657s sys 0m7.602s improvement - 51% -bash-3.00$ time pg_restore -U postgres -p 5435 -m 8 --truncate-before-load -d r ttest /tmp/rtout.pz ... real 6m18.787s user 0m45.361s sys 0m7.811s improvement - 62%