Re: [PROPOSAL] VACUUM Progress Checker. - Mailing list pgsql-hackers

From 大山真実
Subject Re: [PROPOSAL] VACUUM Progress Checker.
Date
Msg-id CAJ_V8TmJG0Z8RPpN9DhTLEnfDxWEUoBQXQRQvQ7mbE47y3X9+Q@mail.gmail.com
Whole thread Raw
In response to Re: [PROPOSAL] VACUUM Progress Checker.  (Vinayak Pokale <vinpokale@gmail.com>)
Responses Re: [PROPOSAL] VACUUM Progress Checker.
List pgsql-hackers
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 <vinpokale@gmail.com>:
Hello,

On Fri, Feb 26, 2016 at 6:19 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

Hi Vinayak,

Thanks for updating the patch! A quick comment:

On 2016/02/26 17:28, pokurev@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

pgsql-hackers by date:

Previous
From: Artur Zakirov
Date:
Subject: Re: Proposal: Generic WAL logical messages
Next
From: Kevin Grittner
Date:
Subject: Re: The plan for FDW-based sharding