Re: [PROPOSAL] VACUUM Progress Checker.

Horiguchi-san,

Thanks for a quick reply, :)

On 2016/03/07 18:18, Kyotaro HORIGUCHI wrote:
> At Mon, 7 Mar 2016 16:16:30 +0900, Amit Langote wrote:
>> On 2016/03/07 13:02, Kyotaro HORIGUCHI wrote:
>>> The 0001-P.. adds the following interface functions.
>>>
>>> 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 thought giving cmdtype and objid each its own slot would make things a
>> little bit clearer than stuffing them into st_progress_param[0] and
>> st_progress_param[1], respectively. Is that what you are suggesting?
>> Although as I've don, a separate field st_command_objid may be a bit too much.
>
> I mentioned only of object id as you seem to take me. The command
> type is essential unlike the target object ids. It is needed by
> all statistics views of this kind to filter required backends.

Yep.

>>> # I think that integer arrays might be needed to be passed as a
>>> # parameter, but it would be the another issue.
>>
>> Didn't really think about it. Maybe we should consider a scenario that
>> would require it.
>
> Imagine to provide a statictics of a vacuum commnad as a
> whole. It will vacuum several relations at once so the view could
> be like the following.
>
> select * from pg_stat_vacuum_command;
> - [ Record 1 ]
> worker_pid : 3243
> command : vacuum full
> rels_scheduled : {16387, 16390, 16393}
> rels_finished : {16384}
> status : Processing 16384, awiting for a lock.
> ..
>
> This needs arrays if we want this but it would be another issue
> as I said.

Oh, I see. This does call for at least some consideration of how to
support variable size parameter values.

By the way, looking at the "status" message in your example, it doesn't
seem like a candidate for evaluation in a CASE..WHEN expression? Maybe,
we should re-introduce[1] a fixed-size char st_progress_message[] field.
Since, ISTM, such a command's internal code is in better position to
compute that kind of message string. IIRC, a reason that was given to not
have such a field was, among other things, the copy overhead of message
strings. But commands like the one in your example, could afford that
much overhead since the frequency of message change would be less and less
compared with the time elapsed between the changes anyway.

>> I think the fixed number of parameters in the form of a fixed-size array
>> is because st_progress_param[] is part of a shared memory structure as
>> discussed before. Although such interface has been roughly modeled on how
>> pg_statistic catalog and pg_stats view or get_attstatsslot() function
>> work, shared memory structures take the place of the catalog, so there are
>> some restrictions (fixed size array being one).
>
> It depends on how easy we take it to widen the parameter slots in
> shared memory:p Anyway I don't stick that since it doesn't make
> a siginificant difference.

Your above example makes me wonder how we can provide for it.

Thanks,
Amit