On Wed, May 25, 2022 at 09:34:51AM -0700, Zhihong Yu wrote:
> Let's look at the sequences of user activity for long running COPY command.
> The user would likely issue queries to pg_stat_progress_copy over time.
> Let's say on Nth invocation, the user sees X tuples copied.
> On (N+1)st invocation, the view returns nothing.
> The user knows that the COPY may have completed - but did the operation
> succeed or end up with some error ?
If I am following this thread correctly and after reading the patch,
that's what the status code of the connection issuing the command is
here for. You have no guarantee either that the status you are trying
to store in the progress view is not going to be quickly overwritten
by a follow-up command, making the window where this information is
available very small in most cases, limiting its value. The window
gets even smaller if the connection that failed the COPY is used in a
connection pooler by a different command.
Hi,
I tend to think that in case of failed COPY command, the user would spend some time trying to find out
why the COPY command failed. The investigation would make the window longer.
The changes in pgstat_progress_end_command() and
pg_stat_get_progress_info() update st_progress_command_target
depending on the command type involved, breaking the existing contract
of those routines, particularly the fact that the progress fields
*should* be reset in an error stack.
I searched the code base for how st_progress_command_target is used.
In case there is subsequent command following the COPY, st_progress_command_target would be set to the Oid
of the subsequent command.
In case there is no subsequent command following the COPY command, it seems leaving st_progress_command_target as
the Oid of the COPY command wouldn't hurt.
Maybe you can let me know what side effect not resetting st_progress_command_target would have.
As an alternative, upon seeing PROGRESS_COMMAND_COPY_DONE, we can transfer the value of
st_progress_command_target to a new field called, say, st_progress_command_previous_target (
and resetting st_progress_command_target as usual).
Please let me know what you think.
Thanks