Thread: Re: [GENERAL] Server process crash - Segmentation fault
Leif Jensen <leif@crysberg.dk> writes:
> Here is a gdb dump of the backtrace at the server process crash. I have also included the code that generates
thesecalls. As mentioned below this specific connection has been used many times before the crash. Also, we are aware
ofthe thread caveat that only using a connection from one thread at a time. Therefore the "strange" connection name
thatincludes both the process id and the thread id. This is for the code to make sure that a connection is only used in
thethread it is meant to.
Hm. The crash looks like it must be because ActiveSnapshot is null
(not set). Since we're doing a FETCH, the active snapshot ought to
be the one saved for the cursor query by DECLARE CURSOR. It looks
like the problem is that pquery.c only bothers to install that as the
active snapshot while calling ExecutorRun, but in this stack trace
we're in ExecutorRewind.
I wonder if it's a bad idea for ExecReScanLimit to be executing
user-defined expressions? But it's been like that for awhile,
and I think we might have a hard time preserving the bounded-sort
optimization if we didn't do that.
Anyway the simple fix would be to ensure we install the query
snapshot as active before calling ExecutorRewind.
One interesting question is why this issue hasn't been seen before;
it seems like it'd not be that hard to hit.
regards, tom lane
Could it be related to the OFFSET part of the statement ? I have another query on the same table without OFFSET, whichseems to work fine. Leif ----- Original Message ----- > Leif Jensen <leif@crysberg.dk> writes: > > Here is a gdb dump of the backtrace at the server process crash. > > I have also included the code that generates these calls. As > > mentioned below this specific connection has been used many times > > before the crash. Also, we are aware of the thread caveat that > > only using a connection from one thread at a time. Therefore the > > "strange" connection name that includes both the process id and > > the thread id. This is for the code to make sure that a > > connection is only used in the thread it is meant to. > > Hm. The crash looks like it must be because ActiveSnapshot is null > (not set). Since we're doing a FETCH, the active snapshot ought to > be the one saved for the cursor query by DECLARE CURSOR. It looks > like the problem is that pquery.c only bothers to install that as the > active snapshot while calling ExecutorRun, but in this stack trace > we're in ExecutorRewind. > > I wonder if it's a bad idea for ExecReScanLimit to be executing > user-defined expressions? But it's been like that for awhile, > and I think we might have a hard time preserving the bounded-sort > optimization if we didn't do that. > > Anyway the simple fix would be to ensure we install the query > snapshot as active before calling ExecutorRewind. > > One interesting question is why this issue hasn't been seen before; > it seems like it'd not be that hard to hit. > > regards, tom lane
Leif Jensen <leif@crysberg.dk> writes:
> Could it be related to the OFFSET part of the statement ? I have another query on the same table without OFFSET,
whichseems to work fine.
Yeah, the specific code path here involves executing a stable (or possibly
immutable) SQL function in a LIMIT or OFFSET clause. I was able to
reproduce the crash like so:
create function foo(int) returns int as 'select $1 limit 1'
language sql stable;
begin;
declare c cursor for select * from int8_tbl limit foo(3);
select * from c;
move backward all in c;
select * from c;
commit;
You might be able to dodge the problem if you can make the SQL function
inline-able (the LIMIT 1 in my example is just to prevent that from
happening). A less appealing alternative is to mark the function
VOLATILE, which I think would also prevent this crash, but might have
negative performance consequences.
If you don't mind building your own PG then you could grab the actual fix
from our git repo; I should have something committed before long.
regards, tom lane
Hi Tom, I already compiled postgreSQL myself and now using 9.3.4, so I would very much like a patch. Where can I find that ? Leif ----- Original Message ----- > Leif Jensen <leif@crysberg.dk> writes: > > Could it be related to the OFFSET part of the statement ? I have > > another query on the same table without OFFSET, which seems to > > work fine. > > Yeah, the specific code path here involves executing a stable (or > possibly > immutable) SQL function in a LIMIT or OFFSET clause. I was able to > reproduce the crash like so: > > create function foo(int) returns int as 'select $1 limit 1' > language sql stable; > > begin; > > declare c cursor for select * from int8_tbl limit foo(3); > > select * from c; > > move backward all in c; > > select * from c; > > commit; > > You might be able to dodge the problem if you can make the SQL > function > inline-able (the LIMIT 1 in my example is just to prevent that from > happening). A less appealing alternative is to mark the function > VOLATILE, which I think would also prevent this crash, but might have > negative performance consequences. > > If you don't mind building your own PG then you could grab the actual > fix > from our git repo; I should have something committed before long. > > regards, tom lane
Leif Jensen <leif@crysberg.dk> writes: > I already compiled postgreSQL myself and now using 9.3.4, so I would very much like a patch. Where can I find that ? 9.3 patch is here: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fc58c39d468587467c7c55b349c28167794eadaf regards, tom lane
On 05/08/2014 06:59 AM, Leif Jensen wrote: > Hi Tom, > > I already compiled postgreSQL myself and now using 9.3.4, so I would very much like a patch. Where can I find that? http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=04e5025be8bbe572e12b19c4ba9e2a8360b8ffe5 > > Leif -- Adrian Klaver adrian.klaver@aklaver.com
On 05/08/2014 07:19 AM, Tom Lane wrote: > Leif Jensen <leif@crysberg.dk> writes: >> I already compiled postgreSQL myself and now using 9.3.4, so I would very much like a patch. Where can I find that? > > 9.3 patch is here: > http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fc58c39d468587467c7c55b349c28167794eadaf Alright, so I obviously linked to the wrong patch because I pointed at HEAD and not REL9_3_STABLE. What I am trying to figure out is what is the distinction between commit and commitdiff? > > regards, tom lane > > -- Adrian Klaver adrian.klaver@aklaver.com
Adrian Klaver <adrian.klaver@aklaver.com> writes: > On 05/08/2014 07:19 AM, Tom Lane wrote: >> 9.3 patch is here: >> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fc58c39d468587467c7c55b349c28167794eadaf > Alright, so I obviously linked to the wrong patch because I pointed at > HEAD and not REL9_3_STABLE. I think those patches are the same, actually, but I was trying to be careful. > What I am trying to figure out is what is > the distinction between commit and commitdiff? The "commitdiff" link shows you the actual diffs in the patch, the other one doesn't. In practice, Leif's going to want to hit the "patch" link anyway to get a clean downloadable patch; so likely what we should have pointed him at is http://git.postgresql.org/gitweb/?p=postgresql.git;a=patch;h=fc58c39d468587467c7c55b349c28167794eadaf I'm just in the habit of looking at the "commitdiff" versions of the web pages as being the best readability/information tradeoff for casual examination of a patch. regards, tom lane
On 05/08/2014 07:43 AM, Tom Lane wrote: > Adrian Klaver <adrian.klaver@aklaver.com> writes: >> On 05/08/2014 07:19 AM, Tom Lane wrote: >>> 9.3 patch is here: >>> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fc58c39d468587467c7c55b349c28167794eadaf > >> Alright, so I obviously linked to the wrong patch because I pointed at >> HEAD and not REL9_3_STABLE. > > I think those patches are the same, actually, but I was trying to be > careful. > >> What I am trying to figure out is what is >> the distinction between commit and commitdiff? > > The "commitdiff" link shows you the actual diffs in the patch, the other > one doesn't. Got it, saves opening the diff for each file. > > In practice, Leif's going to want to hit the "patch" link anyway to get a > clean downloadable patch; so likely what we should have pointed him at is > http://git.postgresql.org/gitweb/?p=postgresql.git;a=patch;h=fc58c39d468587467c7c55b349c28167794eadaf > I'm just in the habit of looking at the "commitdiff" versions of the web > pages as being the best readability/information tradeoff for casual > examination of a patch. Thanks for the explanation. > > regards, tom lane > > -- Adrian Klaver adrian.klaver@aklaver.com
Hello Tom, Adrian Thank you for your help and the patch. Things works nicely for me now :-). Leif ----- Original Message ----- > Adrian Klaver <adrian.klaver@aklaver.com> writes: > > On 05/08/2014 07:19 AM, Tom Lane wrote: > >> 9.3 patch is here: > >> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fc58c39d468587467c7c55b349c28167794eadaf > > > Alright, so I obviously linked to the wrong patch because I pointed > > at > > HEAD and not REL9_3_STABLE. > > I think those patches are the same, actually, but I was trying to be > careful. > > > What I am trying to figure out is what is > > the distinction between commit and commitdiff? > > The "commitdiff" link shows you the actual diffs in the patch, the > other > one doesn't. > > In practice, Leif's going to want to hit the "patch" link anyway to > get a > clean downloadable patch; so likely what we should have pointed him at > is > http://git.postgresql.org/gitweb/?p=postgresql.git;a=patch;h=fc58c39d468587467c7c55b349c28167794eadaf > I'm just in the habit of looking at the "commitdiff" versions of the > web > pages as being the best readability/information tradeoff for casual > examination of a patch. > > regards, tom lane