Re: [HACKERS] create_unique_path and GEQO
Ashutosh Bapat writes: >> Do you have test case, which can reproduce the issue you >> explained above? > No. It would require some surgery in standard_planner() to measure the > memory consumed in the planner context OR build the code with > SHOW_MEMORY_STATS defined and dump memory context statistics and check > planner context memory usage. I don't think I can produce a testcase > quickly right now. But then, I think the problem is quite apparent > from the code inspection alone, esp. comparing what mark_dummy_rel() > does with what create_unique_path() is doing. Yeah. I think the code in mark_dummy_rel is newer and better-thought-out than what's in create_unique_path. It probably makes sense to change over. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] create_unique_path and GEQO
On Fri, Mar 24, 2017 at 11:52 AM, Rushabh Lathia wrote: > > > On Wed, Mar 22, 2017 at 3:43 PM, Ashutosh Bapat > wrote: >> >> Hi, >> In create_unique_path() there's comment >> /* >> * We must ensure path struct and subsidiary data are allocated in >> main >> * planning context; otherwise GEQO memory management causes trouble. >> */ >> oldcontext = MemoryContextSwitchTo(root->planner_cxt); >> >> pathnode = makeNode(UniquePath); >> >> This means that when GEQO resets the memory context, the RelOptInfo >> for which this path is created and may be set to cheapest_unique_path >> goes away, the unique path lingers on in the planner context. >> Shouldn't we instead allocate the path in the same context as the >> RelOptInfo similar to mark_dummy_rel()? >> > > Do you have test case, which can reproduce the issue you > explained above? > No. It would require some surgery in standard_planner() to measure the memory consumed in the planner context OR build the code with SHOW_MEMORY_STATS defined and dump memory context statistics and check planner context memory usage. I don't think I can produce a testcase quickly right now. But then, I think the problem is quite apparent from the code inspection alone, esp. comparing what mark_dummy_rel() does with what create_unique_path() is doing. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] create_unique_path and GEQO
On Wed, Mar 22, 2017 at 3:43 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > Hi, > In create_unique_path() there's comment > /* > * We must ensure path struct and subsidiary data are allocated in main > * planning context; otherwise GEQO memory management causes trouble. > */ > oldcontext = MemoryContextSwitchTo(root->planner_cxt); > > pathnode = makeNode(UniquePath); > > This means that when GEQO resets the memory context, the RelOptInfo > for which this path is created and may be set to cheapest_unique_path > goes away, the unique path lingers on in the planner context. > Shouldn't we instead allocate the path in the same context as the > RelOptInfo similar to mark_dummy_rel()? > > Do you have test case, which can reproduce the issue you explained above? > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Rushabh Lathia
Re: [HACKERS] create_unique_path and GEQO
On Wed, Mar 22, 2017 at 3:43 PM, Ashutosh Bapat wrote: > Hi, > In create_unique_path() there's comment > /* > * We must ensure path struct and subsidiary data are allocated in main > * planning context; otherwise GEQO memory management causes trouble. > */ > oldcontext = MemoryContextSwitchTo(root->planner_cxt); > > pathnode = makeNode(UniquePath); > > This means that when GEQO resets the memory context, the RelOptInfo > for which this path is created and may be set to cheapest_unique_path > goes away, the unique path lingers on in the planner context. > Shouldn't we instead allocate the path in the same context as the > RelOptInfo similar to mark_dummy_rel()? > tried this change as attached patch. I ran make installcheck with geqo_threshold = 2. Only join.sql failed several plans changed, which is expected. There was one difference related to changed output order but that's because of the changed plan. Adding this to the commitfest. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company pg_upath_geqo.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] create_unique_path and GEQO
Hi, In create_unique_path() there's comment /* * We must ensure path struct and subsidiary data are allocated in main * planning context; otherwise GEQO memory management causes trouble. */ oldcontext = MemoryContextSwitchTo(root->planner_cxt); pathnode = makeNode(UniquePath); This means that when GEQO resets the memory context, the RelOptInfo for which this path is created and may be set to cheapest_unique_path goes away, the unique path lingers on in the planner context. Shouldn't we instead allocate the path in the same context as the RelOptInfo similar to mark_dummy_rel()? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers