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

From Syed, Rahila
Subject Re: [PROPOSAL] VACUUM Progress Checker.
Date
Msg-id C3C878A2070C994B9AE61077D46C38468EE65241@MAIL703.KDS.KEANE.COM
Whole thread Raw
In response to Re: [PROPOSAL] VACUUM Progress Checker.  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: [PROPOSAL] VACUUM Progress Checker.  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Re: [PROPOSAL] VACUUM Progress Checker.  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
List pgsql-hackers
Hello,

Please find attached an updated patch.

>Flag isn't reset on error.
Corrected in the attached.

> +    pgstat_reset_activityflag;
>Does this actually compile?
It does compile but with no effect.  It has been corrected.

>snprintf()?  I don't think you need to keep track of schemaname_len at all.
memcpy() has been replaced by snprintf() to avoid calculating schemaname_len.

>In fact, I wonder if you need to send total_pages at all -- surely the client can add both total_heap_pages and
total_index_pagesby itself ... 
This has  been corrected in the attached patch.

>It seems a bit strange that the remaining progress_param entries are not initialized to anything.  Also, why aren't
thenumber of params of each type saved too?   
The number of params for each command remains constant hence it has been hardcoded.

>In the receiving code you check whether each value equals 0, and if it does then report NULL, but imagine vacuuming a
tablewith no indexes where the number of index pages is going to be zero. 
>Shouldn't we display zero there rather than null?
Agree.  IIUC, NULL should rather be used when a value is invalid. But for valid values like 'zero index pages' it is
clearerto display 0. It has been corrected in the attached.  

>This patch lacks a comment somewhere explaining how this whole thing works.
Have added few lines in pgstat.h inside PgBackendStatus struct.

>I believe you don't need this include.
Corrected.

>This not only adds an unnecessary empty line at the end of the struct declaration, but also fails to preserve the
"st_"prefix used in all the other fields 
Corrected.

Thank you,
Rahila Syed


______________________________________________________________________
Disclaimer: This email and any attachments are sent in strictest confidence
for the sole use of the addressee and may contain legally privileged,
confidential, and proprietary data. If you are not the intended recipient,
please advise the sender by replying promptly to this email and then delete
and destroy this email and any attachments without any further use, copying
or forwarding.
Attachment

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Support for N synchronous standby servers - take 2
Next
From: Oleksii Kliukin
Date:
Subject: Re: [ADMIN] Replication slots and isolation levels