Re: Support for REINDEX CONCURRENTLY
От | Fujii Masao |
---|---|
Тема | Re: Support for REINDEX CONCURRENTLY |
Дата | |
Msg-id | CAHGQGwFVAHUFszLuUYizrmkXJE1vCKB-WfHcWF-uuwcBshA_2g@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Support for REINDEX CONCURRENTLY (Michael Paquier <michael.paquier@gmail.com>) |
Список | pgsql-hackers |
On Wed, Jun 19, 2013 at 9:50 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Jun 19, 2013 at 12:36 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >> On Tue, Jun 18, 2013 at 10:53 AM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >>> An updated patch for the toast part is attached. >>> >>> On Tue, Jun 18, 2013 at 3:26 AM, Fujii Masao <masao.fujii@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. > I changed it a little bit in a different way in my latest patch by > adding a sum on all the indexes when getting tidx_blks stats. > >>>> 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. > Not sure about that. The latest code is made such as only one valid > index is present on the toast relation at the same time. > >> >>>> *** 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.... > Well, I have to disagree. I am not able to reproduce it. Which version > did you use? Here is what I get with the latest version of REINDEX > CONCURRENTLY patch... I checked with the following process: Sorry. This is my mistake. Regards, -- Fujii Masao
В списке pgsql-hackers по дате отправления:
Предыдущее
От: David FetterДата:
Сообщение: Re: FILTER for aggregates [was Re: Department of Redundancy Department: makeNode(FuncCall) division]