Re: [HACKERS] [PATCH] New command to monitor progression of longrunning queries - Mailing list pgsql-hackers

On 19.04.2017 17:13, Remi Colinet wrote:
> Maksim,
>
>
> 2017-04-18 20:31 GMT+02:00 Maksim Milyutin <m.milyutin@postgrespro.ru
> <mailto:m.milyutin@postgrespro.ru>>:
>
>     On 18.04.2017 17:39, Remi Colinet wrote:
>
>
>         Regarding the queryDesc state of SQL query upon receiving a
>         request to
>         report its execution progress, it does not bring any issue. The
>         request
>         is noted when the signal is received by the monitored backend.
>         Then, the
>         backend continues its execution code path. When interrupts are
>         checked
>         in the executor code, the request will be dealt.
>
>
>     Yes, interrupts are checked in the CHECK_FOR_INTERRUPTS entries.
>
>         When the request is being dealt, the monitored backend will stop its
>         execution and report the progress of the SQL query. Whatever is the
>         status of the SQL query, progress.c code checks the status and
>         report
>         either that the SQL query does not have a valid status, or
>         otherwise the
>         current execution state of the SQL query.
>
>         SQL query status checking is about:
>         - idle transaction
>         - out of transaction status
>         - null planned statement
>         - utility command
>         - self monitoring
>
>         Other tests can be added if needed to exclude some SQL query
>         state. Such
>         checking is done in void HandleProgressRequest(void).
>         I do not see why a SQL query progression would not be possible
>         in this
>         context. Even when the queryDescc is NULL, we can just report a
>         <idle
>         transaction> output. This is currently the case with the patch
>         suggested.
>
>
>     It's interesting question - how much the active query is in a usable
>     state on the stage of execution. Tom Lane noticed that
>     CHECK_FOR_INTERRUPTS doesn't give us 100% guarantee about full
>     consistency [1].
>
>
> I wonder what you mean about usable state.
>

A usable query state is suitable for analysis, IOW we have consistent 
QueryDesc object. This term was introduced by Tom Lane in [1]. I suppose 
he meant the case when a query fails with error and before transaction 
aborts we bump into *CHECK_FOR_INTERRUPTS* in the place where QueryDesc 
may be inconsistent and further reading from it will give us invalid result.


> Currently, the code suggested tests the queryDesc pointer and all the
> sub nodes pointers in order to detect NULL pointers. When the progress
> report is collected by the backend, this backend does the collect and
> consequently does not run the query. So the query tree is not being
> modified. At this moment, whatever is the query state, we can manage to
> deal with its static state. It is only a tree which could also be just a
> NULL pointer in the most extreme case. Such case is dealt in the current
> code.
>

Perhaps the deep checking of QueryDesc would allow us to consider it as 
consistent.


1. https://www.postgresql.org/message-id/24182.1472745492%40sss.pgh.pa.us

-- 
Maksim Milyutin
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company