On Mon, Jan 28, 2013 at 8:59 PM, Andres Freund <and...@2ndquadrant.com>wrote:
> On 2013-01-28 20:50:21 +0900, Michael Paquier wrote: > > On Mon, Jan 28, 2013 at 8:44 PM, Andres Freund <and...@anarazel.de> > wrote: > > > > > > Another argument that would be enough for a rejection of this patch > by a > > > > committer is the problem of invalid toast indexes that cannot be > removed > > > up > > > > cleanly by an operator. As long as there is not a clean solution for > > > > that... > > > > > > I think that part is relatively easy to fix, I wouldn't worry too > > > much. > > > The more complex part is how to get tuptoaster.c to update the > > > concurrently created index. That's what I worry about. Its not going > > > through the normal executor paths but manually updates the toast > > > index - which means it won't update the indisready && !indisvalid > > > index... > > > > > I included in the patch some stuff to update the reltoastidxid of the > > parent relation of the toast index. Have a look at > > index.c:index_concurrent_swap. The particular case I had in mind was if > > there is a failure of the server during the concurrent reindex of a toast > > index. > > Thats not enough unfortunately. The problem scenario is the following: > > toast table: pg_toast.pg_toast_16384 > toast index (via reltoastidxid): pg_toast.pg_toast_16384_index > REINDEX CONCURRENTLY PHASE #1 > REINDEX CONCURRENTLY PHASE #2 > toast table: pg_toast.pg_toast_16384 > toast index (via reltoastidxid): pg_toast.pg_toast_16384_index, ready & > valid > toast index (via pg_index): pg_toast.pg_toast_16384_index_tmp, ready & > !valid > > If a tuple gets toasted in this state tuptoaster.c will update > 16384_index but not 16384_index_tmp. In normal tables this works because > nodeModifyTable uses ExecInsertIndexTuples which updates all ready > indexes. tuptoaster.c does something different though, it calls > index_insert exactly on the one expected index, not on the other ones. > > Makes sense? > I didn't know toast indexes followed this code path. Thanks for the details. > > > When server restarts, the toast relation will have an invalid index > > and this cannot be dropped by an operator via SQL. > > That requires about two lines of special case code in > RangeVarCallbackForDropRelation, that doesn't seem to be too bad to me. > > I.e. allow the case where its IsSystemClass(classform) && relkind == > RELKIND_INDEX && !indisvalid. > OK, I thought it was more complicated. -- Michael Paquier http://michael.otacoo.com
