On Tue, Dec 5, 2017 at 6:39 PM, Amit Kapila <amit.kapil...@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. The reason I've popped up out of nowhere on this thread to say this is that I just realised that my nearby patch that adds support for Hash instrumentation has the same bug, because I copied the way ExecSortRetrieveInstrumentation() works to make ExecHashRetrieveInstrumentation(). I initially assumed this would run just one after the final loop in a rescan situation but on testing I see that it runs repeatedly, and of course looses intrumentation during rescans. I'll go and fix that. We should choose one approach for both cases. Do you prefer a separate variable for the local copy, or reinstating the pointer into the DSM during ExecXXXReInitializeDSM()? As for how to aggregate the information, isn't it reasonable to show data from the last loop on the basis that it's representative? Summing wouldn't make too much sense, because you didn't use that much memory all at once. -- Thomas Munro http://www.enterprisedb.com