Re: [PROPOSAL] VACUUM Progress Checker.
От | Syed, Rahila |
---|---|
Тема | Re: [PROPOSAL] VACUUM Progress Checker. |
Дата | |
Msg-id | C3C878A2070C994B9AE61077D46C38468EE5E936@MAIL703.KDS.KEANE.COM обсуждение исходный текст |
Ответ на | Re: [PROPOSAL] VACUUM Progress Checker. (Fujii Masao <masao.fujii@gmail.com>) |
Ответы |
Re: [PROPOSAL] VACUUM Progress Checker.
("Syed, Rahila" <Rahila.Syed@nttdata.com>)
Re: [PROPOSAL] VACUUM Progress Checker. ("Syed, Rahila" <Rahila.Syed@nttdata.com>) |
Список | pgsql-hackers |
Hello Fujii-san, >Here are another review comments Thank you for review. Please find attached an updated patch. > You removed some empty lines, for example, in vacuum.h. >Which seems useless to me. Has been corrected in the attached. >Why did you use an array to store the progress information of VACUUM? >I think that it's better to use separate specific variables for them for better code readability, for example, variablesscanned_pages, heap_total_pages, etc. It is designed this way to keep it generic for all the commands which can store different progress parameters in shared memory. >Currently only progress_param_float[0] is used. So there is no need to use an array here. Same as before . This is for later use by other commands. >progress_param_float[0] saves the percetage of VACUUM progress. >But why do we need to save that information into shared memory? >We can just calculate the percentage whenever pg_stat_get_vacuum_progress() is executed, instead. There seems to be no needto save that information. This has been corrected in the attached. >char progress_message[PROGRESS_MESSAGE_LENGTH][N_PROGRESS_PARAM]; >As Sawada pointed out, there is no user of this variable. Have used it to store table name in the updated patch. It can also be used to display index names, current phase of VACUUM. This has not been included in the patch yet to avoid cluttering the display with too much information. >For example, st_activity of autovacuum worker displays "autovacuum ...". >So as Sawada reported, he could not find any entry for autovacuum in pg_stat_vacuum_progress. In the attached patch , I have performed check for autovacuum also. >I think that you should add the flag or something which indicates whether this backend is running VACUUM or not, into PgBackendStatus. >pg_stat_vacuum_progress should display the entries of only backends with that flag set true. This design means that youneed to set the flag to true when starting VACUUM and reset at the end of VACUUM progressing. This design seems better in the sense that we don’t rely on st_activity entry to display progress values. A variable which stores flags for running commands can be created in PgBackendStatus. These flags can be used at the timeof display of progress of particular command. >That is, non-superuser should not be allowed to see the pg_stat_vacuum_progress entry of superuser running VACUUM? This has been included in the updated patch. >This code may cause total_pages and total_heap_pages to decrease while VACUUM is running. Yes. This is because the initial count of total pages to be vacuumed and the pages which are actually vacuumed can vary dependingon visibility of tuples. The pages which are all visible are skipped and hence have been subtracted from total page count. 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.
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Michael PaquierДата:
Сообщение: Re: run pg_rewind on an uncleanly shut down cluster.