From: | Tattsu Yama <yamatattsu(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp>, robertmhaas(at)gmail(dot)com, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, pgsql-hackers(at)postgresql(dot)org, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Subject: | Re: [HACKERS] CLUSTER command progress monitor |
Date: | 2019-09-16 06:26:10 |
Message-ID: | CAOKkKFtN97_LjhLCuqxdrnQQAYLzDwy40L6gtuXiHWkeN97yoA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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 | Content-Type | Size |
---|---|---|
v2_fix_progress_report_for_cluster.patch | application/octet-stream | 37.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2019-09-16 08:31:21 | Re: block-level incremental backup |
Previous Message | Amit Kapila | 2019-09-16 06:16:16 | Re: [HACKERS] [PATCH] pageinspect function to decode infomasks |