On Tue, Apr 10, 2018 at 12:29 PM, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > In contrast, in an indexonly scan you have a single counter and it > doesn't really matter the distribution of fetches done by workers, so it > seems okay to aggregate them all in a single counter. And it being so > simple, it seems reasonable to me to put it in Instrumentation rather > than have a dedicated struct.
I don't have a strong opinion on that. Since we know how many tuples were processed by each worker, knowing how many heap fetches we have on a per-worker basis seems like a good thing to have, too. On the other hand, maybe EXPLAIN (ANALYZE, VERBOSE) would give us that anyway, since it knows about displaying per-worker instrumentation (look for /* Show worker detail */). If it doesn't, then that's probably bad, because I'm pretty sure that when I installed that code the stuff that got displayed for worker instrumentation pretty much matched the stuff that got displayed for overall instrumentation. In any case part of the point is that Instrumentation is supposed to be a generic structure that contains things that are for the most part common to all node types. So what MERGE did to that structure looks like in inadvisable kludge to me. I'd get rid of that and do it a different way rather than propagate the idea that nfilteredX is scratch space that can mean something different in every separate node. >> I was pretty much oblivious to this problem during the initial >> parallel query development and mistakenly assumed that bringing over >> struct Instrumentation was good enough. It emerged late in the game >> that this wasn't really the case, but holding up the whole feature >> because some nodes might have details not reported on a per-worker >> basis didn't really seem to make sense. Whether that was the right >> call is obviously arguable. > > I certainly don't blame you for that. Thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
