Обсуждение: Re: Quadratic planning time for ordered paths over partitioned tables
Hello, On 2025-Jan-22, Alexander Kuzmenkov wrote: > Hi hackers, > > There's currently an unfortunate CPU sink in the planning for > partitioned tables. It happens both for the declarative partitioning, > and for the partitioning through inheritance like we use in > TimescaleDB. > > The gist of it is that there's a linear search of the EM corresponding > to the given child relation in find_ec_member_matching_pathkeys(). I think this is closely related to the work Yuya Watari has been doing at https://postgr.es/m/CAJ2pMkZZHrhgQ5UV0y+STKqx7XVGzENMhL98UbKM-OArvK9dmA@mail.gmail.com Perhaps you could contribute by reviewing that patch series. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Los cuentos de hadas no dan al niño su primera idea sobre los monstruos. Lo que le dan es su primera idea de la posible derrota del monstruo." (G. K. Chesterton)
On Wed, Jan 22, 2025 at 5:36 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > I think this is closely related to the work Yuya Watari has been doing > at > https://postgr.es/m/CAJ2pMkZZHrhgQ5UV0y+STKqx7XVGzENMhL98UbKM-OArvK9dmA@mail.gmail.com > Perhaps you could contribute by reviewing that patch series. Yeah, I'm referencing this one in my email, but it's a series of patches that does a major refactoring changing thousands of lines. I'm not sure when or if it's going to land, do you think applying a quick fix first would make sense? It makes trivial changes in one function.
On 2025-Jan-22, Alexander Kuzmenkov wrote: > On Wed, Jan 22, 2025 at 5:36 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > I think this is closely related to the work Yuya Watari has been doing > > at > > https://postgr.es/m/CAJ2pMkZZHrhgQ5UV0y+STKqx7XVGzENMhL98UbKM-OArvK9dmA@mail.gmail.com > > Perhaps you could contribute by reviewing that patch series. > > Yeah, I'm referencing this one in my email, but it's a series of > patches that does a major refactoring changing thousands of lines. I'm > not sure when or if it's going to land, do you think applying a quick > fix first would make sense? It makes trivial changes in one function. I think it might go faster if you try it out and review it. Which problems did you notice that make you think it's not committable? Maybe you can propose solutions for those problems. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Alexander Kuzmenkov <akuzmenkov@timescale.com> writes:
> Yeah, I'm referencing this one in my email, but it's a series of
> patches that does a major refactoring changing thousands of lines. I'm
> not sure when or if it's going to land, do you think applying a quick
> fix first would make sense? It makes trivial changes in one function.
That "quick fix" seems extremely horrid: since there's only one static
cache variable for all ECs, it's going to help only one very specific
call pattern, which could easily get broken by unrelated changes.
Also, maybe my performance instincts are rooted in old hardware, but
I don't trust integer division with a variable divisor to be cheap.
So ISTM this could as easily make things worse as better. If you
offered something that was less obviously a kluge, maybe we could
use it.
Really I'd think the right place to be fixing this is at a higher
level. Where are the repetitive find_ec_member_matching_expr calls
coming from?
regards, tom lane
On Wed, Jan 22, 2025 at 6:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Really I'd think the right place to be fixing this is at a higher > level. Where are the repetitive find_ec_member_matching_expr calls > coming from? I'm not aware of the repeated calls for the same child, and I mostly see it called once for every partition from prepare_sort_from_pathkeys(). For this case, fixing it at the high level would require something like MergeAppend building a mapping of Pathkey -> EM for all its children upfront, then passing this down to the prepare_sort_from_pathkeys(). The other patch series focuses on this being called from the generate_join_implied_equalities(). That call site could probably use the same approach with building a mapping before generating the partitionwise join paths.
Hi, > > I think this is closely related to the work Yuya Watari has been doing > > at > > https://postgr.es/m/CAJ2pMkZZHrhgQ5UV0y+STKqx7XVGzENMhL98UbKM-OArvK9dmA@mail.gmail.com > > Perhaps you could contribute by reviewing that patch series. > > Yeah, I'm referencing this one in my email, but it's a series of > patches that does a major refactoring changing thousands of lines. I'm > not sure when or if it's going to land, do you think applying a quick > fix first would make sense? It makes trivial changes in one function. It looks like the author keeps the patch set up to date. Although he proposes a complicated refactoring this is better than "quick and dirty fix" as you put it, IMO. So I suggest focusing on this patch set. On top of that somehow I doubt we will find a committer willing to be responsible for merging anything quick and dirty anyway. Did you consider checking if the referenced patchset addresses the issue you described? -- Best regards, Aleksander Alekseev
On 2025-Jan-24, Aleksander Alekseev wrote: > Did you consider checking if the referenced patchset addresses the > issue you described? I ran Kuzmenkov's test case with Watari-san's patch. Planning time goes from 2700ms to 600ms or so. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On Fri, Jan 24, 2025 at 2:37 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > I ran Kuzmenkov's test case with Watari-san's patch. Planning time goes > from 2700ms to 600ms or so. Thank you, good to know that it helps this case as well.