On 19.04.2017 17:13, Remi Colinet wrote:
Maksim,
2017-04-18 20:31 GMT+02:00 Maksim Milyutin <m.milyu...@postgrespro.ru
<mailto:m.milyu...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
