Hi Michael,

>> > Attached file is WIP patch.In my patch, I added "command id" to all
>> APIs of
>> > progress reporting to isolate commands. Therefore, it doesn't allow to
>> > cascade updating system views. And my patch is on WIP so it needs
>> clean-up
>> > and test.
>> > I share it anyway. :)
>>
>> +       if (cmdtype == PROGRESS_COMMAND_INVALID ||
>> beentry->st_progress_command == cmdtype)
>> +       {
>> +               PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
>> +               beentry->st_progress_param[index] = val;
>> +               PGSTAT_END_WRITE_ACTIVITY(beentry);
>> +       }
>> You basically don't need the progress reports if the command ID is
>> invalid, no?
>>
>
>
> Ah, right.
> I'll check and fix that today. :)
>
>

I fixed the patch based on your comment.
Please find attached file.  :)

I should have explained the API changes more. I added cmdtype as a given
parameter for all functions (See below).
Therefore, I suppose that my patch is similar to the following fix as you
mentioned on -hackers.

- Allow only reporting for a given command ID, which would basically
> require to pass down the command ID to progress update APIs and bypass an
> update
> if the command ID provided by caller does not match the existing one
> started (?).


#pgstat.c
pgstat_progress_start_command(ProgressCommandType cmdtype,...)
   - Progress reporter starts when beentry->st_progress_command is
PROGRESS_COMMAND_INVALID

pgstat_progress_end_command(ProgressCommandType cmdtype,...)
   - Progress reporter ends when beentry->st_progress_command equals cmdtype

pgstat_progress_update_param(ProgressCommandType cmdtype,...)  and
pgstat_progress_update_multi_param(ProgressCommandType cmdtype,...)
   - Progress reporter updates parameters if beentry->st_progress_command
equals cmdtype

Note:
cmdtype means the ProgressCommandType below:

# pgstat.h
typedef enum ProgressCommandType
{
    PROGRESS_COMMAND_INVALID,
    PROGRESS_COMMAND_VACUUM,
    PROGRESS_COMMAND_CLUSTER,
    PROGRESS_COMMAND_CREATE_INDEX
} ProgressCommandType;

Thanks,
Tatsuro Yamada

Attachment: v2_fix_progress_report_for_cluster.patch
Description: Binary data

Reply via email to