Обсуждение: find_inheritance_children() and ALTER TABLE NO INHERIT
Currently find_inheritance_children() is smart enough to skip a child table that it finds has been dropped concurrently after it gets a lock on the same. It does so by looking up the child relid in syscache. It seems it should also check if the table is still in the list of children of the parent. Doing so by scanning the pg_inherits(inhparent) index may likely be inefficient. So, how about adding that syscache on pg_inherits(inherelid, inhparent) [1]? I was prompted by a report sometime ago on -general [2] about the "could not find inherited attribute..." error. Also, it was reported as a bug few years ago where locking parent exclusively in ALTER TABLE NO INHERIT as a solution was dismissed for being prone to deadlock issues [3]. Would it be worth the trouble? Thanks, Amit [1] http://www.postgresql.org/message-id/25515.1149652014@sss.pgh.pa.us [2] http://www.postgresql.org/message-id/55BA1A06.1000100@gmail.com [3] http://www.postgresql.org/message-id/19666.1213709303@sss.pgh.pa.us
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes: > Currently find_inheritance_children() is smart enough to skip a child > table that it finds has been dropped concurrently after it gets a lock on > the same. It does so by looking up the child relid in syscache. It seems > it should also check if the table is still in the list of children of the > parent. Doing so by scanning the pg_inherits(inhparent) index may likely > be inefficient. So, how about adding that syscache on > pg_inherits(inherelid, inhparent) [1]? I doubt that a syscache would fix the performance issue there; it wouldn't get referenced enough to be likely to have the desired tuple in cache. I wonder whether we could improve matters by rechecking validity of the pg_inherits tuple (which we saw already and could presumably retain the TID of). There is at least one place where we do something like that now, IIRC. regards, tom lane
On 2015/12/03 13:09, Tom Lane wrote: > Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes: >> Currently find_inheritance_children() is smart enough to skip a child >> table that it finds has been dropped concurrently after it gets a lock on >> the same. It does so by looking up the child relid in syscache. It seems >> it should also check if the table is still in the list of children of the >> parent. Doing so by scanning the pg_inherits(inhparent) index may likely >> be inefficient. So, how about adding that syscache on >> pg_inherits(inherelid, inhparent) [1]? > > I doubt that a syscache would fix the performance issue there; it wouldn't > get referenced enough to be likely to have the desired tuple in cache. Ah, right. > I wonder whether we could improve matters by rechecking validity of the > pg_inherits tuple (which we saw already and could presumably retain the > TID of). There is at least one place where we do something like that now, > IIRC. Given that the generation of child OID list and locking of child tables occur independently, do you mean to collect catalog tuple TIDs along with corresponding OIDs during the catalog scan and recheck them during the locking step? Not sure whether sane but how about performing ordered scan on pg_inherits (systable_getnext_ordered())and using systable_recheck_tuple() in step with it? Does using ordered catalog scan ensure safety against deadlocks that the existing approach of ordered locking of child tables does? Perhaps I'm missing something. Thanks, Amit
On 2015/12/03 15:30, Amit Langote wrote: > On 2015/12/03 13:09, Tom Lane wrote: >> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes: >>> Currently find_inheritance_children() is smart enough to skip a child >>> table that it finds has been dropped concurrently after it gets a lock on >>> the same. It does so by looking up the child relid in syscache. It seems >>> it should also check if the table is still in the list of children of the >>> parent. > >> I wonder whether we could improve matters by rechecking validity of the >> pg_inherits tuple (which we saw already and could presumably retain the >> TID of). There is at least one place where we do something like that now, >> IIRC. > > Not sure whether sane but how about performing ordered scan on pg_inherits > (systable_getnext_ordered())and using systable_recheck_tuple() in step > with it? Does using ordered catalog scan ensure safety against deadlocks > that the existing approach of ordered locking of child tables does? > Perhaps I'm missing something. Just leaving here a patch that does this. It still returns the list in order determined by qsort(), IOW, not in the pg_inherits_parent_index order to avoid broken tests. I could not figure how to do it the way you suggested. Thanks, Amit