Re: [PROPOSAL] VACUUM Progress Checker.

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(at)gmail(dot)com> wrote:

> On Wed, Mar 16, 2016 at 6:44 AM, Rahila Syed <rahilasyed90(at)gmail(dot)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
>