On Fri, Oct 18, 2019 at 10:08 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Oct 17, 2019 at 10:51 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > > > > === > > Don't shut down Gather[Merge] early under Limit. > > > > Revert part of commit 19df1702f5. > > > > Early shutdown was added by that commit so that we could collect > > statistics from workers, but unfortunately it interacted badly with > > rescans. Rescanning a Limit over a Gather node could produce a SEGV > > on 9.6 and resource leak warnings on later releases. By reverting the > > early shutdown code, we might lose statistics in some cases of Limit > > over Gather, but that will require further study to fix. > > > > How about some text like below? I have added slightly different text > to explain the reason for the problem. > > "Early shutdown was added by that commit so that we could collect > statistics from workers, but unfortunately, it interacted badly with > rescans. The problem is that we ended up destroying the parallel > context which is required for rescans. This leads to rescans of a > Limit node over a Gather node to produce unpredictable results as it > tries to access destroyed parallel context. By reverting the early > shutdown code, we might lose statistics in some cases of Limit over > Gather, but that will require further study to fix." > > I am not sure but we can even add a comment in the place where we are > removing some code (in nodeLimit.c) to indicate that 'Ideally we > should shutdown parallel resources here to get the correct stats, but > that would lead to rescans misbehaving when there is a Gather [Merge] > node beneath it. (Explain the reason for misbehavior and the ideas we > discussed in this thread to fix the same) ........." > > I can try to come up with comments in nodeLimit.c on the above lines > if we think that is a good idea? >
I have modified the commit message as proposed above and additionally added comments in nodeLimit.c. I think we should move ahead with this bug-fix patch. If we don't like the comment, it can anyway be improved later. Any suggestions? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
0001-Don-t-shut-down-Gather-Merge-early-under-Limit.patch
Description: Binary data
