On Sat, 10 Aug 2019 at 09:03, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Rowley <david.rowley@2ndquadrant.com> writes:
> > On Fri, 9 Aug 2019 at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I still have hopes for getting rid of es_range_table_array though,
> >> and will look at that tomorrow or so.
>
> > Yes, please. I've measured that to be quite an overhead with large
> > partitioning setups. However, that was with some additional code which
> > didn't lock partitions until it was ... well .... too late... as it
> > turned out. But it seems pretty good to remove code that could be a
> > future bottleneck if we ever manage to do something else with the
> > locking of all partitions during UPDATE/DELETE.
>
> I poked at this, and attached is a patch, but again I'm not seeing
> that there's any real performance-based argument for it. So far
> as I can tell, if we've got a lot of RTEs in an executable plan,
> the bulk of the startup time is going into lock (re) acquisition in
> AcquirePlannerLocks, and/or permissions scanning in ExecCheckRTPerms;
> both of those have to do work for every RTE including ones that
> run-time pruning drops later on. ExecInitRangeTable just isn't on
> the radar.
In the code I tested with locally I ended up with a Bitmapset that
marked which RTEs required permission checks so that
ExecCheckRTPerms() could quickly skip RTEs with requiredPerms == 0.
The Bitmapset was set in the planner. Note:
expand_single_inheritance_child sets childrte->requiredPerms = 0, so
there's nothing to do there for partitions, which is the most likely
reason that the rtable list would be big. Sadly the locking is still
a big overhead even with that fixed. Robert threw around some ideas in
[1], but that seems like a pretty big project.
I don't think removing future bottlenecks is such a bad idea if it can
be done in such a way that the code remains clean. It may serve to
increase our motivation later to solve the remaining issues. We tend
to go to greater lengths when there are more gains, and more gains are
more easily visible by removing more bottlenecks.
Another reason to remove the es_range_table_array is that the reason
it was added in the first place is no longer valid. We'd never have
added it if we had array-based lists back then. (Reading below, it
looks like Alvaro agrees with this too)
[1] https://www.postgresql.org/message-id/CA%2BTgmoYbtm1uuDne3rRp_uNA2RFiBwXX1ngj3RSLxOfc3oS7cQ%40mail.gmail.com
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
