Hi!
I'm interesting this patch and tested it. I found two strange thing.
* Incorrect counting
Reproduce:
1. Client1 execute "VACUUM"
2. Client2 execute "VACUUM"
3. Client3 execute "SELECT * FROM pg_stat_vacuum_progress".
pid | relid | phase | total_heap_blks | current_heap_blkno |
total_index_pages | scanned_index_pages | index_scan_count |
percent_complete
------+-------+---------------+-----------------+--------------------+-------------------+---------------------+------------------+------------------
9267 | 16551 | Scanning Heap | 164151 | 316 |
27422 | 7 | 1 | 0
9764 | 16554 | Scanning Heap | 2 | 2 |
2 | 27422 | 1 | 100
(2 rows)
Client2 is waiting for Clinet1 "VACUUM" but percent_complete of Client2
"VACUUM" is 100.
* Not end VACUUM ANALYZE in spite of "percent_complete=100"
Client_1 execute "VACUUM ANALYZE", then Client_2 execute "SELECT * FROM
pg_stat_vacuum_progress".
pid | relid | phase | total_heap_blks | current_heap_blkno |
total_index_pages | scanned_index_pages | index_scan_count |
percent_complete
------+-------+---------------+-----------------+--------------------+-------------------+---------------------+------------------+------------------
9277 | 16551 | Scanning Heap | 163935 | 163935 |
27422 | 7 | 1 | 100
(1 row
percent_complete is 100 but Client_1 "VACUUM ANALYZE" do not response yet.
Of course, Client_1 is executing analyze after vacuum. But it seem to me
that this confuses users.
If percent_complete becomes 100 that row should be deleted quickly.
Regards,
Masanori Ohyama
NTT Open Source Software Center
2016年2月27日(土) 13:54 Vinayak Pokale <vinpok...@gmail.com>:
> Hello,
>
> On Fri, Feb 26, 2016 at 6:19 PM, Amit Langote <
> langote_amit...@lab.ntt.co.jp> wrote:
>
>>
>> Hi Vinayak,
>>
>> Thanks for updating the patch! A quick comment:
>>
>> On 2016/02/26 17:28, poku...@pm.nttdata.co.jp wrote:
>> >> CREATE VIEW pg_stat_vacuum_progress AS
>> >> SELECT S.s[1] as pid,
>> >> S.s[2] as relid,
>> >> CASE S.s[3]
>> >> WHEN 1 THEN 'Scanning Heap'
>> >> WHEN 2 THEN 'Vacuuming Index and Heap'
>> >> ELSE 'Unknown phase'
>> >> END,
>> >> ....
>> >> FROM pg_stat_get_command_progress(PROGRESS_COMMAND_VACUUM) as S;
>> >>
>> >> # The name of the function could be other than *_command_progress.
>> > The name of function is updated as pg_stat_get_progress_info() and also
>> updated the function.
>> > Updated the pg_stat_vacuum_progress view as suggested.
>>
>> So, pg_stat_get_progress_info() now accepts a parameter to distinguish
>> different commands. I see the following in its definition:
>>
>> + /* Report values for only those backends which are
>> running VACUUM
>> command */
>> + if (cmdtype == COMMAND_LAZY_VACUUM)
>> + {
>> + /*Progress can only be viewed by role member.*/
>> + if (has_privs_of_role(GetUserId(),
>> beentry->st_userid))
>> + {
>> + values[2] =
>> UInt32GetDatum(beentry->st_progress_param[0]);
>> + values[3] =
>> UInt32GetDatum(beentry->st_progress_param[1]);
>> + values[4] =
>> UInt32GetDatum(beentry->st_progress_param[2]);
>> + values[5] =
>> UInt32GetDatum(beentry->st_progress_param[3]);
>> + values[6] =
>> UInt32GetDatum(beentry->st_progress_param[4]);
>> + values[7] =
>> UInt32GetDatum(beentry->st_progress_param[5]);
>> + if (beentry->st_progress_param[1] != 0)
>> + values[8] =
>> Float8GetDatum(beentry->st_progress_param[2] * 100 /
>> beentry->st_progress_param[1]);
>> + else
>> + nulls[8] = true;
>> + }
>> + else
>> + {
>> + nulls[2] = true;
>> + nulls[3] = true;
>> + nulls[4] = true;
>> + nulls[5] = true;
>> + nulls[6] = true;
>> + nulls[7] = true;
>> + nulls[8] = true;
>> + }
>> + }
>>
>> How about doing this in a separate function which takes the command id as
>> parameter and returns an array of values and the number of values (per
>> command id). pg_stat_get_progress_info() then creates values[] and nulls[]
>> arrays from that and returns that as result set. It will be a cleaner
>> separation of activities, perhaps.
>>
>> +1
>
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
