Re: pgsql: Add parallel-aware hash joins.
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@enterprisedb.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-01-24T20:08:09Z
Lists: pgsql-hackers
Hi, On 2018-01-24 14:31:47 -0500, Tom Lane wrote: > However ... if you spend any time looking at the behavior of that, > the hashjoin tests are still problematic. I think my main problem with your arguments is that you basically seem to say that one of the more complex features in postgres can't increase the test time. And I just don't agree with that. If we can reduce some unnecessary overhead (as Thomas iirc has done somwhere nearby) - great, if we can hide the overhead by scheduling the test better or breaking it up - also great. But if that's a good chunk of work I think it's entirely reasonable to not necessarily consider that the best use of time. It doesn't seem too surprising that a test that relies on starting multiple background processes in multiple places will be among the more expensive ones. We clearly would e.g. benefit from being able to reuse workers, to avoid constantly starting/stopping them. > (The overall runtime for "make installcheck-parallel" on this machine > is about 17.3 seconds right now.) The next slowest test script in > the join test's group is "update", at 0.373 seconds; so over 1.5 sec > of the total 17.3 sec runtime is being spent solely in the join script. Might be worth breaking up join a bit, that won't get rid of all the wall time overhead, but should reduce it. Reordering to run parallel to other slow tests might also be worthwhile. > So I continue to maintain that an unreasonable fraction of the total > resources devoted to the regular regression tests is going into these > new hashjoin tests. > > One caveat is that old machines also > > somewhat approximate testing with more instrumentation / debugging > > enabled (say valgrind, CLOBBER_CACHE_ALWAYS, etc). So removing excessive > > test overhead has still quite some benefits. But I definitely do not > > want to lower coverage to achieve it. > > I don't want to lower coverage either. I do want some effort to be > spent on achieving test coverage intelligently, rather than just throwing > large test cases at the code without consideration of the costs. I think this accusation is unfair. Are you really suggesting that nobody else cares about the runtime of the new tests? Just because other people's tradeoffs come down at a somewhat different place, doesn't mean they add tests "without consideration of the costs". > Based on these numbers, it seems like one easy thing we could do to > reduce parallel check time is to split the plpgsql test into several > scripts that could run in parallel. But independently of that, > I think we need to make an effort to push hashjoin's time back down. If we had a dependency based system as I suggested nearby, we could have pg_regress order the tests so that the slowest ones that have dependencies fulfilled are started first... Greetings, Andres Freund
Commits
-
Update obsolete sentence in README.parallel.
- 28e04155f17c 11.0 cited
-
Rewrite ConditionVariableBroadcast() to avoid live-lock.
- aced5a92bf46 11.0 cited
-
Tweak parallel hash join test case in hopes of improving stability.
- 934c7986f4a0 11.0 landed
-
Rename pg_rewind's copy_file_range() to avoid conflict with new linux syscall.
- 3e68686e2c55 11.0 cited
-
Fix some minor errors in new PHJ code.
- 6fcde2406304 11.0 landed
-
Fix EXPLAIN ANALYZE output for Parallel Hash.
- 93ea78b17c47 11.0 landed
-
Fix rare assertion failure in parallel hash join.
- f83040c62a78 11.0 landed
-
Cancel CV sleep during subtransaction abort.
- f3decdc94ea3 10.2 landed
- 59d1e2b95a82 11.0 landed
-
Add parallel-aware hash joins.
- 1804284042e6 11.0 cited
-
Fix EXPLAIN ANALYZE of hash join when the leader doesn't participate.
- 5bcf389ecfd4 11.0 cited
-
Add some regression tests that exercise hash join code.
- fa330f9adf4e 11.0 cited