Re: SegFault on 9.6.14
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, vignesh C <vignesh21@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Jerry Sievers <gsievers19@comcast.net>, Tomas Vondra <tomas.vondra@2ndquadrant.com>,
pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2019-11-18T08:52:44Z
Lists: pgsql-hackers
Attachments
- 0001-Don-t-shut-down-Gather-Merge-early-under-Limit.patch (application/octet-stream) patch 0001
On Fri, Oct 18, 2019 at 10:08 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Oct 17, 2019 at 10:51 AM Thomas Munro <thomas.munro@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
Commits
-
Don't shut down Gather[Merge] early under Limit.
- 1ad0df67c790 9.6.17 landed
- f7ae68aacd9d 10.12 landed
- d0ccfa9d6a3d 11.7 landed
- 1cc3a90c7551 12.2 landed
- 080313f8296f 13.0 landed
-
Prohibit shutting down resources if there is a possibility of back up.
- 2cd0acfdade8 12.0 cited
-
Fix buffer usage stats for parallel nodes.
- 19df1702f5c5 9.6.10 cited
- 85c9d3475e4f 12.0 cited
-
Fix crashes on plans with multiple Gather (Merge) nodes.
- fd7c0fa732d9 11.0 cited
-
Separate reinitialization of shared parallel-scan state from ExecReScan.
- 41b0dd987d44 11.0 cited