Re: Hybrid Hash/Nested Loop joins and caching results from subplans

On Tue, Feb 16, 2021 at 6:16 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Wed, 3 Feb 2021 at 19:51, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > I've attached a spreadsheet with the results of each of the tests.
> >
> > The attached file v13_costing_hacks.patch.txt is the quick and dirty
> > patch I put together to run test 5.
>
> I've attached an updated set of patches. I'd forgotten to run make
> check-world with the 0005 patch and that was making the CF bot
> complain. I'm not intending 0005 for commit in the state that it's
> in, so I've just dropped it.
>
> I've also done some further performance testing with the attached set
> of patched, this time I focused solely on planner performance using
> the Join Order Benchmark. Some of the queries in this benchmark do
> give the planner quite a bit of exercise. Queries such as 29b take my
> 1-year old, fairly powerful AMD hardware about 78 ms to make a plan
> for.
>
> The attached spreadsheet shows the details of the results of these
> tests. Skip to the "Test6 no parallel 100 stats EXPLAIN only" sheet.
>
> To get these results I just ran pgbench for 10 seconds on each query
> prefixed with "EXPLAIN ".
>
> To summarise here, the planner performance gets a fair bit worse with
> the patched code. With master, summing the average planning time over
> each of the queries resulted in a total planning time of 765.7 ms.
> After patching, that went up to 1097.5 ms. I was pretty disappointed
> about that.
>
> On looking into why the performance gets worse, there's a few factors.
> One factor is that I'm adding a new path to consider and if that path
> sticks around then subsequent joins may consider that path. Changing
> things around so I only ever add the best path, the time went down to
> 1067.4 ms. add_path() does tend to ditch inferior paths anyway, so
> this may not really be a good thing to do. Another thing that I picked
> up on was the code that checks if a Result Cache Path is legal to use,
> it must check if the inner side of the join has any volatile
> functions. If I just comment out those checks, then the total planning
> time goes down to 985.6 ms. The estimate_num_groups() call that the
> costing for the ResultCache path must do to estimate the cache hit
> ratio is another factor. When replacing that call with a constant
> value the total planning time goes down to 905.7 ms.
>
> I can see perhaps ways that the volatile function checks could be
> optimised a bit further, but the other stuff really is needed, so it
> appears if we want this, then it seems like the planner is going to
> become slightly slower. That does not exactly fill me with joy. We
> currently have enable_partitionwise_aggregate and
> enable_partitionwise_join which are both disabled by default because
> of the possibility of slowing down the planner. One option could be
> to make enable_resultcache off by default too. I'm not really liking
> the idea of that much though since anyone who leaves the setting that
> way won't ever get any gains from caching the inner side of
> parameterised nested loop results.
>
> The idea I had to speed up the volatile function call checks was along
> similar lines to what parallel query does when it looks for parallel
> unsafe functions in the parse. Right now those checks are only done
> under a few conditions where we think that parallel query might
> actually be used. (See standard_planner()). However, with Result
> Cache, those could be used in many other cases too, so we don't really
> have any means to short circuit those checks. There might be gains to
> be had by checking the parse once rather than having to call
> contains_volatile_functions in the various places we do call it. I
> think both the parallel safety and volatile checks could then be done
> in the same tree traverse. Anyway. I've not done any hacking on this.
> It's just an idea so far.
>
>

> Does anyone have any particular thoughts on the planner slowdown?
>

I used the same JOB test case and testing with 19c.sql, I can get a similar
result with you (There are huge differences between master and v14). I
think the reason is we are trying the result cache path on a very hot line (
nest loop inner path), so the cost will be huge. I see
get_resultcache_path
has some fastpath to not create_resultcache_path, but the limitation looks
too broad. The below is a small adding on it, the planing time can be
reduced from 79ms to 52ms for 19c.sql in my hardware.

+ /*
+ * If the inner path is cheap enough, no bother to try the result
+ * cache path. 20 is just an arbitrary value. This may reduce some
+ * planning time.
+ */
+ if (inner_path->total_cost < 20)
+ return NULL;

> I used imdbpy2sql.py to parse the imdb database files and load the
> data into PostgreSQL. This seemed to work okay apart from the
> movie_info_idx table appeared to be missing. Many of the 113 join
> order benchmark queries need this table.

I followed the steps in [1] and changed something with the attached patch.
At last I got 2367725 rows. But probably you are running into a different
problem since no change is for movie_info_idx table.

[1] https://github.com/gregrahn/join-order-benchmark

--
Best Regards
Andy Fan (https://www.aliyun.com/)