2017-04-19 18:41 GMT+02:00 Maksim Milyutin <m.milyu...@postgrespro.ru>:
> 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. > I could indeed trigger a segmentation fault because the nodes of the tree may be under freeing. Some node may be partially filled for instance. But each node can be checked against null pointer once the monitored backend is no more executing its query and is dumping its progress state. So this is not a big deal in fact. > > > 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 >
