Re: RFC: replace pg_stat_activity.waiting with something more descriptive

On Fri, Mar 11, 2016 at 12:28 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
>
> Committed with some further editing.  In particular, the way you
> determined whether we could safely access the tranche information for
> any given ID was wrong; please check over what I did and make sure
> that isn't also wrong.
>

There are few typos which I have tried to fix with the attached patch.  Can you tell me what was wrong with the way it was done in patch?

@@ -4541,9 +4542,10 @@ AbortSubTransaction(void)

  */

  LWLockReleaseAll();

 

+ pgstat_report_wait_end();

+ pgstat_progress_end_command();

  AbortBufferIO();

  UnlockBuffers();

- pgstat_progress_end_command();

 

  /* Reset WAL record construction state */

  XLogResetInsertion();

@@ -4653,6 +4655,9 @@ AbortSubTransaction(void)

  */

  XactReadOnly = s->prevXactReadOnly;

 

+ /* Report wait end here, when there is no further possibility of wait */

+ pgstat_report_wait_end();

+

  RESUME_INTERRUPTS();

 }

AbortSubTransaction() does call pgstat_report_wait_end() twice, is this intentional? I have kept it in the end because there is a chance that in between API's can again set the state to wait and also by that time we have not released buffer pins and heavyweight locks, so not sure if it makes sense to report wait end at that stage.  I have noticed that in WaitOnLock(), on error the wait end is set, but now again thinking on it, it seems it will be better to set it in AbortTransaction/AbortSubTransaction at end.  What do you think?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com