Rahila Syed <[email protected]> wrote:
> Thanks for suggesting enhancements to the progress reporting framework.
>
> I have comments on the solution's approach.
Thanks.
> /*
> + * Some commands have a sub-command, e.g. REPACK (re)builds indexes. The
> + * target can be different, e.g. when the sub-command builds an index on
> + * TOAST relation.
> + */
> + ProgressCommandType st_progress_command2;
> + Oid st_progress_command_target2;
> + int64 st_progress_param2[PGSTAT_NUM_PROGRESS_PARAM];
>
> This approach only works for a nesting depth of 2, not for 3 or more
> levels of subcommands, for instance.
> I am not aware of a concrete example of such a command but I think we should
> keep the design generic enough to accommodate this.
I thought about all the current command types
typedef enum ProgressCommandType
{
PROGRESS_COMMAND_INVALID,
PROGRESS_COMMAND_VACUUM,
PROGRESS_COMMAND_ANALYZE,
PROGRESS_COMMAND_CREATE_INDEX,
PROGRESS_COMMAND_BASEBACKUP,
PROGRESS_COMMAND_COPY,
PROGRESS_COMMAND_REPACK,
} ProgressCommandType;
The typical problem occurs when REPACK performs reindexing (CREATE_INDEX). I
could only think of one case where the depth would be more than 2: an index
function (executed during the build) running another monitored command. In a
development build (with my patch applied), such a case would fire an assertion
in pgstat_progress_start_command(), while in a production build that innermost
command would only overwrite the status of the CREATE INDEX command. I don't
consider such a crazy case worth more effort.
> Shall we consider reporting the progress counters of a sub-command as
> additional parameters
> in the existing st_progress_param[] array? A top level command can
> define the progress parameters
> based on the number and type of sub-commands it expects to see. This
> way we can even restrict or alter the counters we would like to report
> for a sub-command. This also avoids changing PgBackendStatus every
> time we encounter a command requiring deeper nesting levels.
It might be possible to make sure in some cases that parameter numbers not
used by the main command are used by the sub-command, but I think it would
make coding quite tricky. And if you wanted to be sure that there are no
collisions of parameter numbers, you'd need st_progress_param[] to be twice as
large as the maximum number of parameters per command anyway. So this approach
wouldn't help as long as you're concerned of memory consumption.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
