Thread: [BUGS]

[BUGS]

From
Andres Freund
Date:
Bcc:
Subject: Re: [BUGS] signal 11 segfaults with parallel workers
Reply-To:
In-Reply-To: <CAMAYy4JeYdX3vr+qsmbA9o66+wuSDyPTpsnPbkHNrZbRBYXHcA@mail.gmail.com>

Hi,

(please quote "properly" on pg lists, also don't trim the CC list wholesale)

On 2017-08-08 15:41:38 -0400, Rick Otten wrote:
> > > Whatever it's trying to initialize in _PG_init needs to be done later.
> >
> > Indeed, that's bad. I am adding in CC Ronan who has been working on
> > multicorn. At this stage, I think that you would be better out by
> > disabling parallelism.

> Just to follow up.   The database has not crashed since I disabled
> parallelism.   As a result of that change, some of my queries are running
> dramatically slower, I'm still working on doing what I can to get them back
> up to reasonable performance.   I look forward to a solution that allows
> both FDW extensions and parallel queries to coexist in the same database.

This is going to need a multicorn bugfix. This isn't a postgres bug, and
other FDWs can coexist with parallelism.  Therefore I unfortunately
think we can't really do anything here.

Perhaps, for v11, we should actually make sure there's no memory context
etc set during _PG_init() to catch such problems earlier? It's a bit
nasty to only see them if the shared library is preloaded and/or
parallelism is in use.

Greetings,

Andres Freund


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

[BUGS]

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> Perhaps, for v11, we should actually make sure there's no memory context
> etc set during _PG_init() to catch such problems earlier?

I don't see much of a way to do that in the "typical" case where
the library load happens as a result of a SQL command.  You can't
just say "oh, we're not in a transaction" and then later "wait,
yes we are".

Maybe we should recommend that extension authors test their libraries
in the preload scenario ...
        regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Re: [BUGS] your mail

From
Andres Freund
Date:
On 2017-08-08 16:15:15 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Perhaps, for v11, we should actually make sure there's no memory context
> > etc set during _PG_init() to catch such problems earlier?
> 
> I don't see much of a way to do that in the "typical" case where
> the library load happens as a result of a SQL command.  You can't
> just say "oh, we're not in a transaction" and then later "wait,
> yes we are".

Transaction seems hard, but setting CurrentMemoryContext = NULL during
library load seems quite possible. And that'll cause a lot of code that
assumes an in-progress transaction to fail.

> Maybe we should recommend that extension authors test their libraries
> in the preload scenario ...

That probably won't hurt. A quick search in the docs doesn't turn up any
evidence of us having documented that you shouldn't do database access
in _PG_init()...

Greetings,

Andres Freund


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Re: [BUGS] your mail

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2017-08-08 16:15:15 -0400, Tom Lane wrote:
>> I don't see much of a way to do that in the "typical" case where
>> the library load happens as a result of a SQL command.  You can't
>> just say "oh, we're not in a transaction" and then later "wait,
>> yes we are".

> Transaction seems hard, but setting CurrentMemoryContext = NULL during
> library load seems quite possible. And that'll cause a lot of code that
> assumes an in-progress transaction to fail.

And it would cause a lot of code that *doesn't* assume that to fail,
too.  That has basically nothing to do with not being in a transaction,
so I don't think it would be helpful here.
        regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Re: [BUGS] your mail

From
Andres Freund
Date:
On 2017-08-08 17:12:14 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-08-08 16:15:15 -0400, Tom Lane wrote:
> >> I don't see much of a way to do that in the "typical" case where
> >> the library load happens as a result of a SQL command.  You can't
> >> just say "oh, we're not in a transaction" and then later "wait,
> >> yes we are".
> 
> > Transaction seems hard, but setting CurrentMemoryContext = NULL during
> > library load seems quite possible. And that'll cause a lot of code that
> > assumes an in-progress transaction to fail.
> 
> And it would cause a lot of code that *doesn't* assume that to fail,
> too. That has basically nothing to do with not being in a transaction,
> so I don't think it would be helpful here.

Wouldn't mostsuch code be a bad idea anyway? If it's in transaction
context, you can't rely on the lifetime of allocated memory. If it's
TopMemoryContext, the code should be careful about not leaking.  So
being aware of/explicit about the context used seems advisable.

- Andres


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Re: [BUGS] your mail

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2017-08-08 17:12:14 -0400, Tom Lane wrote:
>> And it would cause a lot of code that *doesn't* assume that to fail,
>> too. That has basically nothing to do with not being in a transaction,
>> so I don't think it would be helpful here.

> Wouldn't mostsuch code be a bad idea anyway?

No, not really.  You're right that a palloc appearing directly in a
_PG_init function is a bit dubious, but that doesn't mean that _PG_init
can't call anything that allocates memory.  Also, since _PG_init is by
definition only called once per process, I do not think that authors
need to be rapped on the knuckles if they leak a small amount of
TopMemoryContext memory.

In any case, the result of doing that would only be that people would
throw in MemoryContextSwitchTo calls, it wouldn't discourage them from
trying to do catalog accesses for instance.
        regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs