On Thu, Mar 10, 2016 at 3:08 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > Hi Vinayak, > > Thanks for the quick review!
Committed 0001 earlier this morning. On 0002: + /* total_index_blks */ + current_index_blks = (BlockNumber *) palloc(nindexes * sizeof(BlockNumber)); + total_index_blks = 0; + for (i = 0; i < nindexes; i++) + { + BlockNumber nblocks = RelationGetNumberOfBlocks(Irel[i]); + + current_index_blks[i] = nblocks; + total_index_blks += nblocks; + } + pgstat_progress_update_param(PROG_PARAM_VAC_IDX_BLKS, total_index_blks); I think this is a bad idea. The value calculated here isn't necessarily accurate, because the number of index blocks can change between the time this is calculated and the time the indexes are actually vacuumed. If a client just wants the length of the indexes in round figures, that's already SQL-visible, and there's little reason to make VACUUM do it all the time whether anyone is looking at the progress information or not. Note that I'm not complaining about the fact that you exposed the heap block count, because in that case you are exposing the actual value that VACUUM is using to guide its work. The client can get the *current* length of the relation, but the value you are exposing gives you the number of blocks *this particular VACUUM intends to scan*. That has some incremental value - but the index information doesn't have the same thing going for it. On 0003: I think you should make this work for all AMs, not just btree, and then consolidate it with 0002. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers