Hello, I have some random comments on this patch addition to Amit's comments.
- Type of the flag of vacuum activity. ACTIVITY_IS_VACUUM is the alone entry in the enum, and the variable to store it is named as *flag. If you don't have any plan to extend this information, the name of this variable would seems better to be something like pgstat_report_vacuum_running and in the type of boolean. - Type of st_progress_param and so. The variable st_progress_param has very generic name but as looking the pg_stat_get_vacuum_progress, every elements of it is in a definite role. If so, the variable should be a struct. st_progress_param_float is currently totally useless. - Definition of progress_message. The definition of progress_message in lazy_scan_heap is "char [PROGRESS_MESSAGE_LENGTH][N_PROGRESS_PARAM]" which looks to be inversed. The following snprintf, | snprintf(progress_message[0], PROGRESS_MESSAGE_LENGTH, "%s", schemaname); certainly destroys the data already stored in it if any. - snprintf() You are so carefully to use snprintf, + snprintf(progress_message[0], PROGRESS_MESSAGE_LENGTH, "%s", schemaname); + strcat(progress_message[0],"."); + strcat(progress_message[0],relname); but the strcats following ruin it. - Calculation of total_heap_pages in lazy_scan_heap. The current code subtracts the number of blocks when skipping_all_visible_blocks is set in two places. But I think it is enough to decrement when skipping. I'll be happy if this can be of any help. regards, At Tue, 10 Nov 2015 14:44:23 +0900, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote in <56418437.5080...@lab.ntt.co.jp> > Thanks for the v6. A few quick comments: > > - duplicate_oids error in HEAD. > > - a compiler warning: > > pgstat.c:2898: warning: no previous prototype for ‘pgstat_reset_activityflag’ > > To fix that use void for empty parameter list - > > -extern void pgstat_reset_activityflag(); > +extern void pgstat_reset_activityflag(void); > > One more change you could do is 's/activityflag/activity_flag/g', which I > guess is a naming related guideline in place. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers