Re: [PROPOSAL] VACUUM Progress Checker.

From: <pokurev(at)pm(dot)nttdata(dot)co(dot)jp>
To: <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, <robertmhaas(at)gmail(dot)com>
Cc: <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, <amitlangote09(at)gmail(dot)com>, <pgsql-hackers(at)postgresql(dot)org>, <bannos(at)nttdata(dot)co(dot)jp>
Subject: Re: [PROPOSAL] VACUUM Progress Checker.
Date: 2016-03-10 08:21:36
Message-ID: 8e09c2fe530d4008aa0019e38c1d5453@MP-MSGSS-MBX007.msg.nttdata.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 5:09 PM
> To: SPS ポクレ ヴィナヤック(三技術) <pokurev(at)pm(dot)nttdata(dot)co(dot)jp>;
> robertmhaas(at)gmail(dot)com
> Cc: horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp; amitlangote09(at)gmail(dot)com; pgsql-
> hackers(at)postgresql(dot)org; SPS 坂野 昌平(三技術) <bannos(at)nttdata(dot)co(dot)jp>
> Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.
>
>
> Hi Vinayak,
>
> Thanks for the quick review!
>
> On 2016/03/10 16:22, pokurev(at)pm(dot)nttdata(dot)co(dot)jp wrote:
> >> On 2016/03/10 14:29, Amit Langote wrote:
> >> Updated patches attached.
> > In 0002-
>
> [ snip ]
>
> > 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);
>
> [ snip ]
>
> > 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;
> >
> > So maybe we can add datname as separate column in
> pg_stat_progress_vacuum, I think it's not required only datid is sufficient.
> > Any comment?
>
> Why do you think showing the name may be unacceptable? Wouldn't that
> be a little more user-friendly? Though maybe, we can follow the
> pg_stat_activity style and have both instead, as you suggest. Attached
> updated version does that.
+1
I think reporting both (datid and datname) is more user-friendly.
Thank you.

Regards,
Vinayak Pokale

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Kuleshov 2016-03-10 08:41:10 utils/misc: Simplify search of end of argv in save_ps_display_args()
Previous Message Regina Obe 2016-03-10 08:21:22 Re: Is there a way around function search_path killing SQL function inlining?