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
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.


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:

Reply via email to