Re: [PROPOSAL] VACUUM Progress Checker.

Поиск
Список
Период
Сортировка
От Rahila Syed
Тема Re: [PROPOSAL] VACUUM Progress Checker.
Дата
Msg-id CAH2L28tbnGCzJh12CDKtS5cCQjMy=R9xPk=V5U_m7gRTJ0z50A@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PROPOSAL] VACUUM Progress Checker.  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: [PROPOSAL] VACUUM Progress Checker.  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
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 (heap_fill_tuple) checks this tuple descriptor before inserting the value into the tuple store. It finds the attribute type pass by reference and hence it treats the value as a pointer when it is not and thus it fails at the time of memcpy.

This happens because appropriate conversion function is not employed while storing the value of that particular attribute into the values array before copying it into tuple store.

-                               values[i+3] = UInt32GetDatum(beentry->st_progress_param[i]);
+                               values[i+3] = Int64GetDatum(beentry->st_progress_param[i]);           


Attached patch fixes this.

Thank you,
Rahila Syed

On Wed, Mar 16, 2016 at 11:30 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Mar 16, 2016 at 6:44 AM, Rahila Syed <rahilasyed90@gmail.com> 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
>>accurate, fixing some stylistic stuff, arranging to update multiple
>>counters automatically where it could otherwise produce confusion,
>>moving the new view near similar ones in the file, reformatting it to
>>follow the style of the rest of the file, exposing the counter
>>#defines via a header file in case extensions want to use them, and
>>overhauling and substantially expanding the documentation
>
> We have following lines,
>
>         /* report that everything is scanned and vacuumed */
>         pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED,
> blkno);
>         pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED,
> blkno);
>
>
> which appear before final vacuum cycle happens for any remaining dead tuples
> which may span few pages if I am not mistaken.
>
> IMO, reporting final count of heap_blks_scanned is correct here, but
> reporting final heap_blks_vacuumed can happen after the final VACUUM cycle
> for more accuracy.

You are quite right.  Good catch.  Fixed that, and applied Vinayak's
patch too, and fixed another mistake I saw while I was at it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: WIP: Access method extendability
Следующее
От: Robert Haas
Дата:
Сообщение: Re: [PROPOSAL] VACUUM Progress Checker.