Re: [PROPOSAL] VACUUM Progress Checker.

Hi Amit,

Thank you for updating the patch.

> -----Original Message-----
> From: Amit Langote [mailto:Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp]
> Sent: Thursday, March 10, 2016 3:36 PM
> To: Robert Haas <robertmhaas(at)gmail(dot)com>
> Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>; Amit Langote
> <amitlangote09(at)gmail(dot)com>; SPS ポクレ ヴィナヤック(三技術)
> <pokurev(at)pm(dot)nttdata(dot)co(dot)jp>; pgsql-hackers(at)postgresql(dot)org; SPS 坂野 昌
> 平(三技術) <bannos(at)nttdata(dot)co(dot)jp>
> Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.
>
> On 2016/03/10 14:29, Amit Langote wrote:
> > I rebased remainder patches (attached).
> >
> > 0001 is a small patch to fix issues reported by Tomas and Vinayak.
> > 0002 and 0003 are WIP patches to implement progress reporting for
> vacuum.
>
> Oops, in 0002, I wrongly joined with pg_class in the definition of
> pg_stat_progress_vacuum to output the schema-qualified name of the table
> being vacuumed. That means we need to connect to the correct database,
> which is undesirable. Updated version fixes that (shows database name and
> relid). You may also have noticed that I said pg_stat_progress_vacuum, not
> pg_stat_vacuum_progress (IMHO, the former is a better name).
>
> Updated patches attached.
In 0002-
+CREATE VIEW pg_stat_progress_vacuum AS
+ SELECT
+ S.pid AS pid,
+ D.datname AS database,
+ S.relid AS relid,
.
.
.
.
+ FROM pg_database D, pg_stat_get_progress_info('VACUUM') AS S
+ WHERE S.datid = D.oid;
I think we need to use datid instead of datname.
Robert added datid in pg_stat_get_progress_info() and we are using that function here.
+values[1] = ObjectIdGetDatum(beentry->st_databaseid);

+DATA(insert OID = 3318 ( pg_stat_get_progress_info PGNSP PGUID 12 1 100 0 0 f f f f f t s r 1 0 2249 "25" "{25,23,26,26,20,20,20,20,20,20,20,20,20,20}" "{i,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{cmdtype,pid,datid,relid,param1,param2,param3,param4,param5,param6,param7,param8,param9,param10}" _null_ _null_ pg_stat_get_progress_info _null_ _null_ _null_ ));

So I think it's better to report datid not datname.
The definition of view is simply like:
+CREATE VIEW pg_stat_progress_vacuum AS
+ SELECT
+ S.pid AS pid,
+ S.datid AS datid,
+ S.relid AS relid,
+ CASE S.param1
+ WHEN 1 THEN 'scanning heap'
+ WHEN 2 THEN 'vacuuming indexes'
+ WHEN 3 THEN 'vacuuming heap'
+ WHEN 4 THEN 'cleanup'
+ ELSE 'unknown phase'
+ END AS processing_phase,
+ S.param2 AS total_heap_blocks,
+ S.param3 AS current_heap_block,
+ S.param4 AS total_index_blocks,
+ S.param5 AS index_blocks_done,
+ S.param6 AS index_vacuum_count,
+ CASE S.param2
+ WHEN 0 THEN round(100.0, 2)
+ ELSE round((S.param3 + 1) * 100.0 / S.param2, 2)
+ END AS percent_done
+ FROM pg_stat_get_progress_info('VACUUM') AS S;

In the pg_stat_activity view, datid and datname are the separate columns. So maybe we can add datname as separate column in pg_stat_progress_vacuum, but I think it's not required only datid is sufficient.
Any comment?

Regards,
Vinayak Pokale