Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
Hubert Lubaczewski <depesz@depesz.com>, pgsql-hackers mailing list <pgsql-hackers@postgresql.org>
Date: 2017-12-05T11:02:00Z
Lists: pgsql-hackers
Attachments
- fix_accum_instr_parallel_workers_v4.patch (application/octet-stream) patch v4
On Tue, Dec 5, 2017 at 1:29 PM, Thomas Munro <thomas.munro@enterprisedb.com> wrote: > On Tue, Dec 5, 2017 at 8:49 PM, Thomas Munro > <thomas.munro@enterprisedb.com> wrote: >> On Tue, Dec 5, 2017 at 6:39 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: >>> I have one another observation in the somewhat related area. From the >>> code, it looks like we might have some problem with displaying sort >>> info for workers for rescans. I think the problem with the sortinfo >>> is that it initializes shared info with local memory in >>> ExecSortRetrieveInstrumentation after which it won't be able to access >>> the values in shared memory changed by workers in rescans. We might >>> be able to fix it by having some local_info same as sahred_info in >>> sort node. But the main problem is how do we accumulate stats for >>> workers across rescans. The type of sort method can change across >>> rescans. We might be able to accumulate the size of Memory though, >>> but not sure if that is right. I think though this appears to be >>> somewhat related to the problem being discussed in this thread, it can >>> be dealt separately if we want to fix it. >> >> Yeah, that's broken. ExecSortRetrieveInstrumentation() is run for >> each loop, and after the first loop we've lost track of the pointer >> into shared memory because we replaced it with palloc'd copy. We >> could do what you said, or we could reinstate the pointer into the DSM >> in ExecSortReInitializeDSM() by looking it up in the TOC. > > Or would it be an option to change the time > ExecXXXRetrieveInstrumentation() is called so it is run only once? > To me, that doesn't sound like a bad option. I think if do so, then we don't even need to reinitialize the shared sort stats. I think something, like attached, should work if we want to go this route. We can add regression test if this is what we think is a good idea. Having said that, one problem I see doing thing this way is that in general, we will display the accumulated stats of each worker, but for sort or some other special nodes (like hash), we will show the information of only last loop. I am not sure if that is a matter of concern, but if we want to do this way, then probably we should explain this in documentation as well. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Commits
-
Back-port fix for accumulation of parallel worker instrumentation.
- 2157a61b5aa3 9.6.7 landed
-
Test instrumentation of Hash nodes with parallel query.
- 7d3583ad9ae5 11.0 landed
-
Try again to fix accumulation of parallel worker instrumentation.
- 72567f617460 10.2 landed
- 8526bcb2df76 11.0 landed
-
Revert "Fix accumulation of parallel worker instrumentation."
- 7b3f85073c8d 10.2 landed
- 1d6fb35ad629 11.0 landed
-
Fix accumulation of parallel worker instrumentation.
- 778e78ae9fa5 10.2 landed
- 2c09a5c12a66 11.0 landed