Re: RFC: Logging plan of the running query
От | torikoshia |
---|---|
Тема | Re: RFC: Logging plan of the running query |
Дата | |
Msg-id | 33455325d4bc6ec5a04f8cd2d460fdb9@oss.nttdata.com обсуждение исходный текст |
Ответ на | Re: RFC: Logging plan of the running query (Ekaterina Sokolova <e.sokolova@postgrespro.ru>) |
Ответы |
Re: RFC: Logging plan of the running query
(torikoshia <torikoshia@oss.nttdata.com>)
|
Список | pgsql-hackers |
On 2021-10-13 23:28, Ekaterina Sokolova wrote: > Hi, hackers! > > • The last version of patch is correct applied. It changes 8 files > from /src/backend, and 9 other files. > > • I have 1 error and 1 warning during compilation on Mac. > > explain.c:4985:25: error: implicit declaration of function > 'GetLockMethodLocalHash' is invalid in C99 > [-Werror,-Wimplicit-function-declaration] > hash_seq_init(&status, GetLockMethodLocalHash()); > explain.c:4985:25: warning: incompatible integer to pointer conversion > passing 'int' to parameter of type 'HTAB *' (aka 'struct HTAB *') > [-Wint-conversion] > hash_seq_init(&status, GetLockMethodLocalHash()); > > This error doesn't appear at my second machine with Ubuntu. > > I found the reason. You delete #ifdef USE_ASSERT_CHECKING from > implementation of function GetLockMethodLocalHash(void), but this > ifdef exists around function declaration. There may be a situation, > when implementation exists without declaration, so files with using of > function produce errors. I create new version of patch with fix of > this problem. Thanks for fixing that! > I'm agree that seeing the details of a query is a useful feature, but > I have several doubts: > > 1) There are lots of changes of core's code. But not all users need > this functionality. So adding this functionality like extension seemed > more reasonable. It would be good if we can implement this feature in an extension, but as pg_query_state extension needs applying patches to PostgreSQL, I think this kind of feature needs PostgreSQL core modification. IMHO extensions which need core modification are not easy to use in production environments.. > 2) There are many tools available to monitor the status of a query. > How much do we need another one? For example: > • pg_stat_progress_* is set of views with current status of > ANALYZE, CREATE INDEX, VACUUM, CLUSTER, COPY, Base Backup. You can > find it in PostgreSQL documentation [1]. > • pg_query_state is contrib with 2 patches for core (I hope > someday Community will support adding this patches to PostgreSQL). It > contains function with printing table with pid, full query text, plan > and current progress of every node like momentary EXPLAIN ANALYSE for > SELECT, UPDATE, INSERT, DELETE. So it supports every flags and formats > of EXPLAIN. You can find current version of pg_query_state on github > [2]. Also I found old discussion about first version of it in > Community [3]. Thanks for introducing the extension! I only took a quick look at pg_query_state, I have some questions. pg_query_state seems using shm_mq to expose the plan information, but there was a discussion that this kind of architecture would be tricky to do properly [1]. Does pg_query_state handle difficulties listed on the discussion? It seems the caller of the pg_query_state() has to wait until the target process pushes the plan information into shared memory, can it lead to deadlock situations? I came up with this question because when trying to make a view for memory contexts of other backends, we encountered deadlock situations. After all, we gave up view design and adopted sending signal and logging. Some of the comments of [3] seem useful for my patch, I'm going to consider them. Thanks! > 3) Have you measured the overload of your feature? It would be really > interesting to know the changes in speed and performance. I haven't measured it yet, but I believe that the overhead for backends which are not called pg_log_current_plan() would be slight since the patch just adds the logic for saving QueryDesc on ExecutorRun(). The overhead for backends which is called pg_log_current_plan() might not slight, but since the target process are assumed dealing with long-running query and the user want to know its plan, its overhead would be worth the cost. > Thank you for working on this issue. I would be glad to continue to > follow the development of this issue. Thanks for your help! -- Regards, -- Atsushi Torikoshi NTT DATA CORPORATION
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Kyotaro HoriguchiДата:
Сообщение: Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Следующее
От: Amit LangoteДата:
Сообщение: Re: Partition Check not updated when insert into a partition