Re: Speeding up INSERTs and UPDATEs to partitioned tables

On Fri, Nov 16, 2018 at 11:40 AM Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> On 2018-Nov-15, Amit Langote wrote:
>
> > Maybe name it PARTITION_INIT_ALLOCSIZE (dropping the ROUTING from it), or
> > PROUTE_INIT_ALLOCSIZE, to make it clear that it's only allocation size?
>
> Here's a proposed delta on v17 0001. Most importantly, I noticed that
> the hashed subplans stuff didn't actually work, because the hash API was
> not being used correctly. So the search in the hash would never return
> a hit, and we'd create RRIs for those partitions again. To fix this, I
> added a new struct to hold hash entries.

I'm a bit surprised that you found that the hash table didn't work,
because I remember having checked by attaching gdb that it works when
I was hacking on my own delta patch, but I may have been looking at
too many things.

> I think this merits that the performance tests be redone. (Unless I
> misunderstand, this shouldn't change the performance of INSERT, only
> that of UPDATE.)

Actually, I don't remember seeing performance tests done with UPDATEs
on this thread.

Since we don't needlessly scan *all* subplan result rels anymore,
maybe this removes a good deal of overhead for UPDATEs that update
partition key.

> On the subject of the ArraySpace routines, I decided to drop them and
> instead do the re-allocations on the places where they were needed.
> In the original code there were two places for the partitions array, but
> both did the same thing so it made sense to create a separate routine to
> do it instead (ExecRememberPartitionRel), and do the allocation there.
> Just out of custom I moved the palloc to appear at the same place as the
> repalloc, and after doing so it became obvious that we were
> over-allocating memory for the PartitionDispatchData pointer --
> allocating the size for the whole struct instead of just the pointer.
>
> (I renamed the "allocsize" struct members to "max", as is customary.)

These changes look good to me.

> I added CHECK_FOR_INTERRUPTS to the ExecFindPartition loop. It
> shouldn't be useful if the code is correct, but if there are bugs it's
> better to be able to interrupt infinite loops :-)

Good measure. :)

> I reindented the comment atop PartitionTupleRouting. The other way was
> just too unwieldy.
>
> Let me know what you think. Regression tests still pass for me.

Overall, it looks good to me.

Thanks,
Amit