Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-25 Thread Rahila Syed
>Oops. I forgot to credit you in the commit message. Sorry about that. :-( No problem :). Thanks for the commit. Thank you, Rahila Syed

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-24 Thread Amit Langote
On 2016/03/24 22:01, Robert Haas wrote: > On Thu, Mar 24, 2016 at 8:45 AM, Rahila Syed wrote: >> >> - values[i+3] = >> UInt32GetDatum(beentry->st_progress_param[i]); >> + values[i+3] = >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-24 Thread Robert Haas
On Thu, Mar 24, 2016 at 9:01 AM, Robert Haas wrote: > On Thu, Mar 24, 2016 at 8:45 AM, Rahila Syed wrote: >> Server crash was reported on running vacuum progress checker view on 32-bit >> machine. >> Please find attached a fix for the same. >> >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-24 Thread Robert Haas
On Thu, Mar 24, 2016 at 8:45 AM, Rahila Syed wrote: > Server crash was reported on running vacuum progress checker view on 32-bit > machine. > Please find attached a fix for the same. > > Crash was because 32 bit machine considers int8 as being passed by reference > while

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-24 Thread Rahila Syed
Hello, Server crash was reported on running vacuum progress checker view on 32-bit machine. Please find attached a fix for the same. Crash was because 32 bit machine considers int8 as being passed by reference while creating the tuple descriptor. At the time of filling the tuple store, the code

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-20 Thread Robert Haas
On Wed, Mar 16, 2016 at 6:44 AM, Rahila Syed wrote: >>Sorta. Committed after renaming what you called heap blocks vacuumed >>back to heap blocks scanned, adding heap blocks vacuumed, removing the >>overall progress meter which I don't believe will be anything close to

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-16 Thread Rahila Syed
>Sorta. Committed after renaming what you called heap blocks vacuumed >back to heap blocks scanned, adding heap blocks vacuumed, removing the >overall progress meter which I don't believe will be anything close to >accurate, fixing some stylistic stuff, arranging to update multiple >counters

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-16 Thread pokurev
nttdata.co.jp>; Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp>; pgsql-hackers@postgresql.org; SPS 坂野 > 昌平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker. > > On Tue, Mar 15, 2016 at 1:16 AM, Amit Langote > <langote_am

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-15 Thread Amit Langote
On 2016/03/16 2:33, Robert Haas wrote: > On Tue, Mar 15, 2016 at 1:16 AM, Amit Langote > wrote: >> On 2016/03/15 3:41, Robert Haas wrote: >>> Well, I think you need to study the index AMs and figure this out. >> >> OK. I tried to put calls to the callback in

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-15 Thread Robert Haas
On Tue, Mar 15, 2016 at 1:16 AM, Amit Langote wrote: > On 2016/03/15 3:41, Robert Haas wrote: >> On Sat, Mar 12, 2016 at 7:49 AM, Amit Langote >> wrote: >>> Instead, the attached patch adds a IndexBulkDeleteProgressCallback >>> which AMs

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-14 Thread Amit Langote
On 2016/03/15 3:41, Robert Haas wrote: > On Sat, Mar 12, 2016 at 7:49 AM, Amit Langote wrote: >> Instead, the attached patch adds a IndexBulkDeleteProgressCallback >> which AMs should call for every block that's read (say, right before a >> call to ReadBufferExtended) as

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-14 Thread Robert Haas
On Sat, Mar 12, 2016 at 7:49 AM, Amit Langote wrote: > On Fri, Mar 11, 2016 at 2:31 PM, Amit Langote > wrote: >> On 2016/03/11 13:16, Robert Haas wrote: >>> On Thu, Mar 10, 2016 at 9:04 PM, Amit Langote >>>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-14 Thread Amit Langote
Hi, Thanks for taking a look at the patch. On Mon, Mar 14, 2016 at 6:55 PM, Rahila Syed wrote: > Hello, > > While I am still looking at this WIP patch, I had one suggestion. > > Instead of making changes in the index AM API can we have a call to update > the shared state

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-14 Thread Rahila Syed
Hello, While I am still looking at this WIP patch, I had one suggestion. Instead of making changes in the index AM API can we have a call to update the shared state using pgstat_progress* API directly from specific index level code? Like pgstat_count_index_scan(rel) call from _bt_first does.

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-12 Thread Jim Nasby
On 3/10/16 7:48 AM, Robert Haas wrote: I think the problem is that you can't show the name of a non-global SQL object (such as a relation) unless the object is in the current database. Many of the views in the first group are database-local views, while things like pg_locks span all databases.

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-12 Thread Amit Langote
On Fri, Mar 11, 2016 at 2:31 PM, Amit Langote wrote: > On 2016/03/11 13:16, Robert Haas wrote: >> On Thu, Mar 10, 2016 at 9:04 PM, Amit Langote >> wrote: >>> So, from what I understand here, we should not put total count of index >>>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Amit Langote
On 2016/03/11 13:16, Robert Haas wrote: > On Thu, Mar 10, 2016 at 9:04 PM, Amit Langote > wrote: >> So, from what I understand here, we should not put total count of index >> pages into st_progress_param; rather, have the client (reading >> pg_stat_progress_vacuum)

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 9:04 PM, Amit Langote wrote: > So, from what I understand here, we should not put total count of index > pages into st_progress_param; rather, have the client (reading > pg_stat_progress_vacuum) derive it using pg_indexes_size() (?), as and >

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Amit Langote
On 2016/03/10 23:29, Robert Haas wrote: > On Thu, Mar 10, 2016 at 3:08 AM, Amit Langote > wrote: >> Hi Vinayak, >> >> Thanks for the quick review! > > Committed 0001 earlier this morning. Thanks! > On 0002: > > + /* total_index_blks */ > +

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 3:08 AM, Amit Langote 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)); +

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Robert Haas
On Thu, Mar 10, 2016 at 6:10 AM, Kyotaro HORIGUCHI wrote: > So, I have looked into system_views.sql and picked up what > catalogs/views shows objects in such way, that is, showing both > object id and its name. > > Show by name: pg_policies, pg_rules,

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Kyotaro HORIGUCHI
Hi, At Thu, 10 Mar 2016 08:21:36 +, wrote in <8e09c2fe530d4008aa0019e38c1d5...@mp-msgss-mbx007.msg.nttdata.co.jp> > > > 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

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread pokurev
hi.kyot...@lab.ntt.co.jp; amitlangot...@gmail.com; pgsql- > hack...@postgresql.org; SPS 坂野 昌平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker. > > > Hi Vinayak, > > Thanks for the quick review! > > On 2016/03/10 16:22, poku...@pm.nt

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-10 Thread Amit Langote
Hi Vinayak, Thanks for the quick review! On 2016/03/10 16:22, poku...@pm.nttdata.co.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

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread pokurev
ntt.co.jp>; Amit Langote > <amitlangot...@gmail.com>; SPS ポクレ ヴィナヤック(三技術) > <poku...@pm.nttdata.co.jp>; pgsql-hackers@postgresql.org; SPS 坂野 昌 > 平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker. > > On 2016/03/10 14:29

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread Amit Langote
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

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread Amit Langote
On 2016/03/10 2:16, Robert Haas wrote: > On Wed, Mar 9, 2016 at 2:37 AM, Amit Langote > wrote: >> On 2016/03/09 10:11, Amit Langote wrote: >>> The attached revision addresses above and one of Horiguchi-san's comments >>> in his email yesterday. >> >> I fixed one

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread pokurev
ntt.co.jp>; Amit Langote > <amitlangot...@gmail.com>; SPS ポクレ ヴィナヤック(三技術) > <poku...@pm.nttdata.co.jp>; pgsql-hackers@postgresql.org; SPS 坂野 昌 > 平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker. > > On Wed, Mar 9, 2016 a

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread Tomas Vondra
Hi, On Wed, 2016-03-09 at 12:16 -0500, Robert Haas wrote: > On Wed, Mar 9, 2016 at 2:37 AM, Amit Langote > wrote: > > On 2016/03/09 10:11, Amit Langote wrote: > >> The attached revision addresses above and one of Horiguchi-san's comments > >> in his email

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-09 Thread Robert Haas
On Wed, Mar 9, 2016 at 2:37 AM, Amit Langote wrote: > On 2016/03/09 10:11, Amit Langote wrote: >> The attached revision addresses above and one of Horiguchi-san's comments >> in his email yesterday. > > I fixed one more issue in 0002 per Horiguchi-san's comment.

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread pokurev
ヴィ > ナヤック(三技術) <poku...@pm.nttdata.co.jp>; pgsql- > hack...@postgresql.org; SPS 坂野 昌平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker. > > > On 2016/03/08 18:19, Kyotaro HORIGUCHI wrote: > >> +WHEN 0 THEN 100:

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
On 2016/03/09 10:11, Amit Langote wrote: > The attached revision addresses above and one of Horiguchi-san's comments > in his email yesterday. I fixed one more issue in 0002 per Horiguchi-san's comment. Sorry about so many versions. Thanks, Amit >From 9473230af72e0a0e3b60a8ddf1922698f7f17145

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
> On 2016/03/08 18:19, Kyotaro HORIGUCHI wrote: >> + WHEN 0 THEN 100::numeric(5, 2) >> + ELSE ((S.param3 + 1)::numeric / S.param2 * >> 100)::numeric(5, 2) >> >> This usage of numeric seems overkill to me. > > Hmm, how could this rather be written? OK,

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
Horiguchi-san, Thanks for the review! On 2016/03/08 18:19, Kyotaro HORIGUCHI wrote: >> Updated versions attached. >> >> * changed st_progress_param to int64 and so did the argument of >> pgstat_progress_update_param(). Likewise changed param1..param10 of >> pg_stat_get_progress_info()'s output

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
On 2016/03/09 9:22, Kyotaro HORIGUCHI wrote: >> On Wed, Mar 9, 2016 at 12:24 AM, Robert Haas wrote: >>> This patch has been worked on by so many people and reviewed by so >>> many people that I can't keep track of who should be credited when it >>> gets committed. Could

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
On 2016/03/09 0:24, Robert Haas wrote: > On Tue, Mar 8, 2016 at 3:02 AM, Amit Langote > wrote: >> Updated versions attached. >> >> * changed st_progress_param to int64 and so did the argument of >> pgstat_progress_update_param(). Likewise changed param1..param10 of

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Kyotaro HORIGUCHI
At Wed, 9 Mar 2016 01:16:26 +0900, Amit Langote wrote in > On Wed, Mar 9, 2016 at 12:24 AM, Robert Haas wrote: > > This patch has been worked on by so many people and reviewed by

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
On Wed, Mar 9, 2016 at 12:24 AM, Robert Haas wrote: > This patch has been worked on by so many people and reviewed by so > many people that I can't keep track of who should be credited when it > gets committed. Could someone provide a list of author(s) and > reviewer(s)?

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Robert Haas
On Tue, Mar 8, 2016 at 3:02 AM, Amit Langote wrote: > Updated versions attached. > > * changed st_progress_param to int64 and so did the argument of > pgstat_progress_update_param(). Likewise changed param1..param10 of > pg_stat_get_progress_info()'s output columns

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Kyotaro HORIGUCHI
You're so quick. At Tue, 8 Mar 2016 17:02:24 +0900, Amit Langote wrote in <56de8710.4070...@lab.ntt.co.jp> > On 2016/03/07 23:48, Robert Haas wrote: > >> I don't like to treat the target object id differently from other > >> parameters. It could not be needed at

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-08 Thread Amit Langote
On 2016/03/07 23:48, Robert Haas wrote: > On Sun, Mar 6, 2016 at 11:02 PM, Kyotaro HORIGUCHI > wrote: >> The 0001-P.. adds the following interface functions. >> >> +extern void pgstat_progress_set_command(BackendCommandType cmdtype); >> +extern void

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-07 Thread Robert Haas
On Sun, Mar 6, 2016 at 11:02 PM, Kyotaro HORIGUCHI wrote: > At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote > wrote in >> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-07 Thread Amit Langote
On 2016/03/07 19:11, Amit Langote wrote: > we should re-introduce[1] a fixed-size char st_progress_message[] field. Sorry, that [1] does not refer to anything, just a leftover from my draft. I thought I had a link handy for an email where some sort of justification was given as to why

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-07 Thread Amit Langote
Horiguchi-san, Thanks for a quick reply, :) On 2016/03/07 18:18, Kyotaro HORIGUCHI wrote: > At Mon, 7 Mar 2016 16:16:30 +0900, Amit Langote wrote: >> On 2016/03/07 13:02, Kyotaro HORIGUCHI wrote: >>> The 0001-P.. adds the following interface functions. >>> >>> I don't like to treat the target

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-07 Thread Kyotaro HORIGUCHI
Hi, Amit. At Mon, 7 Mar 2016 16:16:30 +0900, Amit Langote wrote in <56dd2ace.5050...@lab.ntt.co.jp> > > Horiguchi-san, > > Thanks a lot for taking a look! > > On 2016/03/07 13:02, Kyotaro HORIGUCHI wrote: > > At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-06 Thread Amit Langote
Horiguchi-san, Thanks a lot for taking a look! On 2016/03/07 13:02, Kyotaro HORIGUCHI wrote: > At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote wrote: >> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote wrote: >>> So, I took the Vinayak's latest patch and rewrote it a

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-06 Thread Kyotaro HORIGUCHI
Hi, Thank you for the patch. At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote wrote in > On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote wrote: > > So, I took the Vinayak's latest

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-06 Thread pokurev
c: SPS ポクレ ヴィナヤック(三技術) <poku...@pm.nttdata.co.jp>; > Amit Langote <langote_amit...@lab.ntt.co.jp>; Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp>; pgsql-hackers@postgresql.org; SPS 坂野 > 昌平(三技術) <ban...@nttdata.co.jp> > Subject: Re: [HACKERS] [PROPOSAL] VAC

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-04 Thread Amit Langote
On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote wrote: > So, I took the Vinayak's latest patch and rewrote it a little ... > I broke it into two: > > 0001-Provide-a-way-for-utility-commands-to-report-progres.patch > 0002-Implement-progress-reporting-for-VACUUM-command.patch

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-04 Thread Amit Langote
On Sat, Mar 5, 2016 at 7:11 AM, Robert Haas wrote: > On Fri, Feb 26, 2016 at 3:28 AM, wrote: >> Thank you for your comments. >> Please find attached patch addressing following comments. > > I'm positive I've said this at least once before while

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-04 Thread Robert Haas
On Fri, Feb 26, 2016 at 3:28 AM, wrote: > Thank you for your comments. > Please find attached patch addressing following comments. > >>As I might have written upthread, transferring the whole string >>as a progress message is useless at least in this scenario. Since

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-03-01 Thread Kyotaro HORIGUCHI
Hello, thank for testing this. At Sat, 27 Feb 2016 17:19:05 +, 大山真実 wrote in > Hi! > > I'm interesting this patch and tested it. I found two strange thing. > > * Incorrect counting > >

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-27 Thread 大山真実
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

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-26 Thread Vinayak Pokale
Hello, On Fri, Feb 26, 2016 at 6:19 PM, Amit Langote 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, > >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-26 Thread Amit Langote
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' >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-26 Thread pokurev
Hello, Thank you for your comments. Please find attached patch addressing following comments. >As I might have written upthread, transferring the whole string >as a progress message is useless at least in this scenario. Since >they are a set of fixed messages, each of them can be represented

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-16 Thread Amit Langote
Hi, On 2016/02/16 18:25, Kyotaro HORIGUCHI wrote: > At Tue, 16 Feb 2016 10:39:27 +0900, Amit Langote wrote: >> On 2016/02/15 20:21, Kyotaro HORIGUCHI wrote: >>> CREATE FUNCTION >>> pg_stat_get_command_progress(IN cmdtype integer) >>> RETURNS SETOF integer[] as $$ >>> >>> SELECT * from

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-16 Thread Kyotaro HORIGUCHI
Hello, At Tue, 16 Feb 2016 10:39:27 +0900, Amit Langote wrote in <56c27dcf.7020...@lab.ntt.co.jp> > > Hello, > > On 2016/02/15 20:21, Kyotaro HORIGUCHI wrote: > > At Mon, 8 Feb 2016 11:37:17 +0900, Amit Langote wrote: > >> On 2016/02/05 17:15,

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-15 Thread Amit Langote
Hello, On 2016/02/15 20:21, Kyotaro HORIGUCHI wrote: > At Mon, 8 Feb 2016 11:37:17 +0900, Amit Langote wrote: >> On 2016/02/05 17:15, poku...@pm.nttdata.co.jp wrote: >>> Please find attached updated patch. [ ... ] >> >> Instead of passing the array of char *'s, why not just pass a single char

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-15 Thread Kyotaro HORIGUCHI
Hello, At Mon, 8 Feb 2016 11:37:17 +0900, Amit Langote wrote in <56b7ff5d.7030...@lab.ntt.co.jp> > > Hi Vinayak, > > Thanks for updating the patch, a couple of comments: > > On 2016/02/05 17:15, poku...@pm.nttdata.co.jp wrote: > > Hello, > > > > Please find

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-08 Thread Alvaro Herrera
Since things are clearly still moving here, I closed it as returned-with-feedback. Please submit to the next CF so that we don't lose it. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-07 Thread Amit Langote
Hi Vinayak, Thanks for updating the patch, a couple of comments: On 2016/02/05 17:15, poku...@pm.nttdata.co.jp wrote: > Hello, > > Please find attached updated patch. >> The point of having pgstat_report_progress_update_counter() is so that >> you can efficiently update a single counter

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-02-05 Thread pokurev
Hello, Please find attached updated patch. >The point of having pgstat_report_progress_update_counter() is so that >you can efficiently update a single counter without having to update >everything, when only one counter has changed. But here you are >calling this function a whole bunch of

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-31 Thread Amit Langote
On 2016/01/29 21:02, Rahila Syed wrote: >> Okay, I agree that reporting just the current blkno is simple and good >> enough. How about numbers of what we're going to report as the "Vacuuming >> Index and Heap" phase? I guess we can still keep the scanned_index_pages >> and index_scan_count So we

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-29 Thread Robert Haas
On Fri, Jan 29, 2016 at 7:02 AM, Rahila Syed wrote: > Apart from these, as suggested in [1] , finer grained reporting from index > vacuuming phase can provide better insight. Currently we report number of > blocks processed once at the end of vacuuming of each index. >

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-29 Thread Rahila Syed
>Okay, I agree that reporting just the current blkno is simple and good >enough. How about numbers of what we're going to report as the "Vacuuming >Index and Heap" phase? I guess we can still keep the scanned_index_pages >and index_scan_count So we have: >+CREATE VIEW pg_stat_vacuum_progress AS >+

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-28 Thread Amit Langote
On 2016/01/28 23:53, Robert Haas wrote: > On Thu, Jan 28, 2016 at 8:41 AM, Amit Langote wrote: >> Or keep scanned_heap_pages as is and add a skipped_pages (or >> skipped_heap_pages). I guess the latter would be updated not only for >> all visible skipped pages but also

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-28 Thread Rahila Syed
>+if(!scan_all) >+scanned_heap_pages = scanned_heap_pages + >next_not_all_visible_block; >I don't want to be too much of a stickler for details here, but it >seems to me that this is an outright lie. Initially the scanned_heap_pages were meant to report just

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-28 Thread Amit Langote
Hi, On Thu, Jan 28, 2016 at 7:38 PM, Rahila Syed wrote: >>+if(!scan_all) >>+scanned_heap_pages = scanned_heap_pages + >>next_not_all_visible_block; > >>I don't want to be too much of a stickler for details here, but it >>seems to me

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-28 Thread Robert Haas
On Thu, Jan 28, 2016 at 8:41 AM, Amit Langote wrote: > Or keep scanned_heap_pages as is and add a skipped_pages (or > skipped_heap_pages). I guess the latter would be updated not only for > all visible skipped pages but also pin skipped pages. That is, > updating its

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-27 Thread Robert Haas
On Tue, Jan 26, 2016 at 11:37 PM, Vinayak Pokale wrote: > Hi, > > Please find attached updated patch with an updated interface. Well, this isn't right. You've got this sort of thing: +scanned_index_pages += RelationGetNumberOfBlocks(Irel[i]); +/*

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-26 Thread Vinayak Pokale
Hi, Please find attached updated patch with an updated interface. On Jan 26, 2016 11:22 AM, "Vinayak Pokale" wrote: > > Hi Amit, > > Thank you for reviewing the patch. > > On Jan 26, 2016 9:51 AM, "Amit Langote" wrote: > > > > > > Hi Vinayak,

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-25 Thread Amit Langote
Hi Vinayak, On 2016/01/25 20:58, Vinayak Pokale wrote: > Hi, > > Please find attached updated patch with an updated interface. > Thanks for updating the patch. > I added the below interface to update the > scanned_heap_pages,scanned_index_pages and index_scan_count only. > void

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-25 Thread Vinayak Pokale
Hi, Please find attached updated patch with an updated interface. I added the below interface to update the scanned_heap_pages,scanned_index_pages and index_scan_count only. void pgstat_report_progress_scanned_pages(int num_of_int, uint32 *progress_scanned_pages_param) Other interface functions

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-25 Thread Vinayak Pokale
Hi Amit, Thank you for reviewing the patch. On Jan 26, 2016 9:51 AM, "Amit Langote" wrote: > > > Hi Vinayak, > > On 2016/01/25 20:58, Vinayak Pokale wrote: > > Hi, > > > > Please find attached updated patch with an updated interface. > > > > Thanks for updating the

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-12 Thread Amit Langote
On 2016/01/12 11:28, Vinayak Pokale wrote: > On Jan 12, 2016 11:22 AM, "Amit Langote" > wrote: >> >> On 2016/01/12 10:30, Amit Langote wrote: >>> I'm slightly concerned that the latest patch doesn't incorporate any >>> revisions to the original pgstat interface per

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-11 Thread Amit Langote
On 2016/01/08 21:20, Rahila Syed wrote: >> I suspect you need to create a new CF entry for this patch in CF 2016-01. > > Unless I am missing something, there seems to be no entry for this patch > into CF 2016-01 page: https://commitfest.postgresql.org/8/. > Regrettably, we have exceeded the

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-11 Thread Vinayak Pokale
On Jan 12, 2016 11:22 AM, "Amit Langote" wrote: > > On 2016/01/12 10:30, Amit Langote wrote: > > I'm slightly concerned that the latest patch doesn't incorporate any > > revisions to the original pgstat interface per Robert's comments in [1]. > > I meant to say

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-11 Thread Amit Langote
On 2016/01/12 10:30, Amit Langote wrote: > I'm slightly concerned that the latest patch doesn't incorporate any > revisions to the original pgstat interface per Robert's comments in [1]. I meant to say "originally proposed pgstat interface on this thread". Thanks, Amit -- Sent via

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-11 Thread Vinayak Pokale
Hi Sudhir, On Jan 7, 2016 3:02 AM, "Sudhir Lonkar-2" wrote: > > Hello, > >Please find attached patch addressing following comments > I have tested this patch. > It is showing empty (null) in phase column of pg_stat_vacuum_progress, when > I switched to a unprivileged

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-08 Thread Rahila Syed
>I suspect you need to create a new CF entry for this patch in CF 2016-01. Unless I am missing something, there seems to be no entry for this patch into CF 2016-01 page: https://commitfest.postgresql.org/8/. Regrettably, we have exceeded the deadline to add the patch into this commitfest. Is

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2016-01-06 Thread Sudhir Lonkar-2
Hello, >Please find attached patch addressing following comments I have tested this patch. It is showing empty (null) in phase column of pg_stat_vacuum_progress, when I switched to a unprivileged user. In the previous patch, it is showing in phase column. Thanks and Regards, Sudhir Lonkar --

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-27 Thread Amit Langote
Hi Vinayak, On 2015/12/25 21:46, Vinayak Pokale wrote: > Hi, > Please find attached patch addressing following comments. > >> The relation OID should be reported and not its name. In case of a >> relation rename that would not be cool for tracking, and most users >> are surely going to join

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-25 Thread Vinayak Pokale
Hi, Please find attached patch addressing following comments. >The relation OID should be reported and not its name. In case of a >relation rename that would not be cool for tracking, and most users >are surely going to join with other system tables using it. The relation OID is reported instead

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-11 Thread Robert Haas
On Fri, Dec 11, 2015 at 1:25 AM, Michael Paquier wrote: >> For another thing, there are definitely going to be >> some people that want the detailed information - and I can practically >> guarantee that if we don't make it available, at least one person will >> write a

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Amit Langote
On 2015/12/10 20:46, Michael Paquier wrote: > On Thu, Dec 10, 2015 at 7:23 PM, Amit Langote > wrote: >> AIUI, the counts published via stats collector are updated asynchronously >> w.r.t. operations they count and mostly as aggregate figures. For example, >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Amit Langote
On 2015/12/10 15:28, Michael Paquier wrote: > On Thu, Dec 10, 2015 at 4:40 AM, Robert Haas wrote: >> It's going to be *really* important that this facility provides a >> lightweight way of updating progress, so I think this whole API is >> badly designed. VACUUM, for

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Michael Paquier
On Thu, Dec 10, 2015 at 7:23 PM, Amit Langote wrote: > On 2015/12/10 15:28, Michael Paquier wrote: >> - The progress tracking facility adds a whole level of complexity for >> very little gain, and IMO this should *not* be part of PgBackendStatus >> because in most

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Robert Haas
On Thu, Dec 10, 2015 at 9:49 AM, Tom Lane wrote: > Robert Haas writes: >> Oh, please, no. Gosh, this is supposed to be a lightweight facility! >> Just have a chunk of shared memory and write the data in there. If >> you try to feed this through the

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Kyotaro HORIGUCHI
Sorry, I misunderstood the meaning of PgStat_*. At Fri, 11 Dec 2015 09:41:04 +0900, Amit Langote wrote in <566a1ba0.70...@lab.ntt.co.jp> > > As far as I understand it, the basic reason why this patch exists is > > to allow a DBA to have a hint of the progress of a

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Amit Langote
On 2015/12/11 14:41, Kyotaro HORIGUCHI wrote: > Sorry, I misunderstood the meaning of PgStat_*. I should've just said "messages to the stats collector" instead of "PgStat_Msg's". > > At Fri, 11 Dec 2015 09:41:04 +0900, Amit Langote > wrote >>> As far as I

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Michael Paquier
On Fri, Dec 11, 2015 at 12:59 AM, Robert Haas wrote: > On Thu, Dec 10, 2015 at 9:49 AM, Tom Lane wrote: >> Robert Haas writes: >>> Oh, please, no. Gosh, this is supposed to be a lightweight facility! >>> Just have a chunk of

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Robert Haas
On Thu, Dec 10, 2015 at 6:46 AM, Michael Paquier wrote: > On Thu, Dec 10, 2015 at 7:23 PM, Amit Langote > wrote: >> On 2015/12/10 15:28, Michael Paquier wrote: >>> - The progress tracking facility adds a whole level of complexity for >>>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-10 Thread Tom Lane
Robert Haas writes: > Oh, please, no. Gosh, this is supposed to be a lightweight facility! > Just have a chunk of shared memory and write the data in there. If > you try to feed this through the stats collector you're going to > increase the overhead by 100x or more, and

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-09 Thread Michael Paquier
On Thu, Dec 10, 2015 at 4:40 AM, Robert Haas wrote: > On Mon, Dec 7, 2015 at 2:41 AM, Amit Langote > wrote: >> Hm, I guess progress messages would not change that much over the course >> of a long-running command. How about we pass only the

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-09 Thread Amit Langote
On 2015/12/10 4:40, Robert Haas wrote: > It's going to be *really* important that this facility provides a > lightweight way of updating progress, so I think this whole API is > badly designed. VACUUM, for example, is going to want a way to update > the individual counter for the number of pages

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-09 Thread Kyotaro HORIGUCHI
Hello, At Thu, 10 Dec 2015 15:28:14 +0900, Michael Paquier wrote in

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-09 Thread Robert Haas
On Mon, Dec 7, 2015 at 2:41 AM, Amit Langote wrote: > On 2015/12/03 19:05, Kyotaro HORIGUCHI wrote: >> At Thu, 3 Dec 2015 16:24:32 +0900, Amit Langote >> wrote >>> By the way, there are some non-st_progress_* fields that I was

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-06 Thread Amit Langote
Hi, On 2015/12/03 19:05, Kyotaro HORIGUCHI wrote: > At Thu, 3 Dec 2015 16:24:32 +0900, Amit Langote > wrote >> By the way, there are some non-st_progress_* fields that I was talking >> about in my previous message. For example, st_activity_flag (which I have >>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

2015-12-03 Thread Kyotaro HORIGUCHI
Hello, At Thu, 3 Dec 2015 16:24:32 +0900, Amit Langote wrote in <565fee30.8010...@lab.ntt.co.jp> > > Agreed. The patch already separates integer values and texts. > > And re-reviewing the patch, there's no fields necessary to be > > passed as string. > > > >

  1   2   3   >