On Tue, Jun 18, 2013 at 10:53 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > An updated patch for the toast part is attached. > > On Tue, Jun 18, 2013 at 3:26 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >> Here are the review comments of the removal_of_reltoastidxid patch. >> I've not completed the review yet, but I'd like to post the current comments >> before going to bed ;) >> >> *** a/src/backend/catalog/system_views.sql >> - pg_stat_get_blocks_fetched(X.oid) - >> - pg_stat_get_blocks_hit(X.oid) AS tidx_blks_read, >> - pg_stat_get_blocks_hit(X.oid) AS tidx_blks_hit >> + pg_stat_get_blocks_fetched(X.indrelid) - >> + pg_stat_get_blocks_hit(X.indrelid) AS tidx_blks_read, >> + pg_stat_get_blocks_hit(X.indrelid) AS tidx_blks_hit >> >> ISTM that X.indrelid indicates the TOAST table not the TOAST index. >> Shouldn't we use X.indexrelid instead of X.indrelid? > Indeed good catch! We need in this case the statistics on the index > and here I used the table OID. Btw, I also noticed that as multiple > indexes may be involved for a given toast relation, it makes sense to > actually calculate tidx_blks_read and tidx_blks_hit as the sum of all > stats of the indexes.
Yep. You seem to need to change X.indexrelid to X.indrelid in GROUP clause. Otherwise, you may get two rows of the same table from pg_statio_all_tables. >> doc/src/sgml/diskusage.sgml >>> There will be one index on the >>> <acronym>TOAST</> table, if present. + table (see <xref linkend="storage-toast">). There will be one valid index + on the <acronym>TOAST</> table, if present. There also might be indexes When I used gdb and tracked the code path of concurrent reindex patch, I found it's possible that more than one *valid* toast indexes appear. Those multiple valid toast indexes are viewable, for example, from pg_indexes. I'm not sure whether this is the bug of concurrent reindex patch. But if it's not, you seem to need to change the above description again. >> *** a/src/bin/pg_dump/pg_dump.c >> + "SELECT c.reltoastrelid, >> t.indexrelid " >> "FROM pg_catalog.pg_class c LEFT >> JOIN " >> - "pg_catalog.pg_class t ON >> (c.reltoastrelid = t.oid) " >> - "WHERE c.oid = >> '%u'::pg_catalog.oid;", >> + "pg_catalog.pg_index t ON >> (c.reltoastrelid = t.indrelid) " >> + "WHERE c.oid = >> '%u'::pg_catalog.oid AND t.indisvalid " >> + "LIMIT 1", >> >> Is there the case where TOAST table has more than one *valid* indexes? > I just rechecked the patch and is answer is no. The concurrent index > is set as valid inside the same transaction as swap. So only the > backend performing the swap will be able to see two valid toast > indexes at the same time. According to my quick gdb testing, this seems not to be true.... Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
