Re: [PROPOSAL] VACUUM Progress Checker.

Hi, Thank you for the patch.

At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote <amitlangote09(at)gmail(dot)com> wrote in <CA+HiwqHTeuqWMc+ktneGqFdJMRXD=syncgU0914TVXaahOF56g(at)mail(dot)gmail(dot)com>
> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > So, I took the Vinayak's latest patch and rewrote it a little
> ...
> > I broke it into two:
> >
> > 0001-Provide-a-way-for-utility-commands-to-report-progres.patch
> > 0002-Implement-progress-reporting-for-VACUUM-command.patch
>
> Oops, unamended commit messages in those patches are misleading. So,
> please find attached corrected versions.

The 0001-P.. adds the following interface functions.

+extern void pgstat_progress_set_command(BackendCommandType cmdtype);
+extern void pgstat_progress_set_command_target(Oid objid);
+extern void pgstat_progress_update_param(int index, uint32 val);
+extern void pgstat_reset_local_progress(void);
+extern int pgstat_progress_get_num_param(BackendCommandType cmdtype);

I don't like to treat the target object id differently from other
parameters. It could not be needed at all, or could be needed two
or more in contrast. Although oids are not guaranteed to fit
uint32, we have already stored BlockNumber there.

# I think that integer arrays might be needed to be passed as a
# parameter, but it would be the another issue.

pg_stat_get_progress_info returns a tuple with 10 integer columns
(plus an object id). The reason why I suggested use of an integer
array is that it allows the API to serve arbitrary number of
parmeters without a modification of API, and array indexes are
coloreless than any concrete names. Howerver I don't stick to
that if we agree that it is ok to have fixed number of paremters.

pgstat_progress_get_num_param looks not good in the aspect of
genericity. I'd like to define it as an integer array by idexed
by the command type if it is needed. However it seems to me to be
enough that pg_stat_get_progress_info always returns 10 integers
regardless of what the numbers are for. The user sql function,
pg_stat_vacuum_progress as the first user, knows how many numbers
should be read for its work. It reads zeroes safely even if it
reads more than what the producer side offered (unless it tries
to divide something with it).

What do you think about this?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center