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

On Fri, Mar 11, 2016 at 12:28 AM, Robert Haas <robertmhaas(at)gmail(dot)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