Re: partitioning code reorganization
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Amit Langote <amitlangote09@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-04-17T20:18:50Z
Lists: pgsql-hackers
Amit Langote wrote: > 0001-Make-copying-of-cached-partitioning-info-more-con.patch > 0002-Cache-all-partitioning-info-under-one-memory-cont.patch > 0003-Cache-partsupfunc-separately-from-PartitionKey.patch I'd rather not do these patches now, unless there is some pressing reason to (eg. some bug crops up). I know Tom dislikes some of the code being used for relcache, but I don't know of any actual bugs; my proposal is put these patches to sleep until pg12 opens. > I think going the way of 0001 might seem counter to what we may *really* > want to do in this regard, which is to make it so that we can use (keep > around) the pointer to partition info in relcache, instead of copying the > information in it piece by piece for every query. Robert's email from a > couple of months ago (that he also recently mentioned) brought this up wrt > to relcache data usage within the planner: I hadn't seen that discussion, thanks. Some braindump follows. Re 0002 though I didn't carefully review the patch, I worry that an error partway some of these routines would leave you with either a long-lived memory leak (or permanent!), or dangling pointers in the relcache entry. If that is so, you can probably fix it by having a memcxt callback that resets the other partition-related members from the relcache entry being processed, so that they are rebuilt; then reset the memcxt. (make the memcxt always exist, for simplicity). The other thing I was interested in was to build the partitioning info on demand rather than immediately when the relcache entry is created. I think there must be tons of places where we want the relcache entry for other reasons and we don't want to waste time building partition desc/key uselessly. Another line of thought. Instead of copying anything, did you consider the idea of using refcounted structs? Rough sketch: in RelationGetPartitionDesc, build struct if relcache doesn't have one, otherwise just increment pincount in existing struct; create new ReleasePartitionDesc which decrements pincount, and if pincount is 0 and not referenced from relcache entry, free the struct (memcxt delete?); if relcache inval is received, detach partitiondesc from relcache entry and build a new one. So old pointers continue to work, until the release. Maybe need to get ResourceOwner involved to make sure we don't accidentally lose track of a PartitionDesc with pincout != 0. This is all pg12 anyway. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Reorganize partitioning code
- da6f3e45ddb6 11.0 landed