Re-thing PG_MODULE_MAGIC

Hey,

I just noticed (the hard way) that in 8.2CVS, the PG_MODULE_MAGIC header is
now *required* for all loadable modules. This includes non-pg modules,
such as Solaris' libumem (performance improvement for malloc).

FATAL:  incompatible library "/usr/lib/libumem.so": missing magic block
HINT:  Extension libraries are now required to use the  
PG_MODULE_MAGIC macro.

Should we really be requiring PG_MODULE_MAGIC for stuff that wasn't built
with PG? This seems like a way to break a lot of people's software.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> I just noticed (the hard way) that in 8.2CVS, the PG_MODULE_MAGIC header is
> now *required* for all loadable modules. This includes non-pg modules,
> such as Solaris' libumem (performance improvement for malloc).

What is libumem and why are you trying to load it as a dynamic module?

> Should we really be requiring PG_MODULE_MAGIC for stuff that wasn't built
> with PG?

If we don't require it then there's precious little point in having it
at all. But why don't you link libumem when building the postmaster,
if you want it?

regards, tom lane

All,

> FATAL:  incompatible library "/usr/lib/libumem.so": missing magic block
> HINT:  Extension libraries are now required to use the  
> PG_MODULE_MAGIC macro.
>
> Should we really be requiring PG_MODULE_MAGIC for stuff that wasn't
> built with PG? This seems like a way to break a lot of people's
> software.

Never mind, Neil C. just pointed out that we shouldn't be using
preload_libraries for libumem anyway -- it's sloppy. However, I do think
that we should brace ourselves for a slew of bug reports; if the Sun
testing guys are misusing preload_libraries this way, they won't be the
only onese.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus wrote:

>
>Never mind, Neil C. just pointed out that we shouldn't be using
>preload_libraries for libumem anyway -- it's sloppy. However, I do think
>that we should brace ourselves for a slew of bug reports; if the Sun
>testing guys are misusing preload_libraries this way, they won't be the
>only onese.
>
>
>

Worth a comment in the config file?

cheers

andrew

Andrew,

> Worth a comment in the config file?

Possibly. Or the docs.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus wrote:
> Andrew,
>
> > Worth a comment in the config file?
>
> Possibly. Or the docs.

SGML config docs updated:

The <quote>magic block</> present in all PostgreSQL-supported libraries
is checked to guarantee compatibility. For this reason, non-PostgreSQL
libraries cannot be loaded in this way.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce,

> The <quote>magic block</> present in all PostgreSQL-supported libraries
> is checked to guarantee compatibility. For this reason, non-PostgreSQL
> libraries cannot be loaded in this way.

you need "must be" before "present".

Also, this is in runtime-config.sgml?

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus wrote:
> Bruce,
>
> > The <quote>magic block</> present in all PostgreSQL-supported libraries
> > is checked to guarantee compatibility. For this reason, non-PostgreSQL
> > libraries cannot be loaded in this way.
>
> you need "must be" before "present".
>
> Also, this is in runtime-config.sgml?

Uh, the point is not that they need a magic block (though they do), but
rather that the magic block is always checked.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Josh Berkus wrote:
>>> The <quote>magic block</> present in all PostgreSQL-supported libraries
>>> is checked to guarantee compatibility. For this reason, non-PostgreSQL
>>> libraries cannot be loaded in this way.
>>
>> you need "must be" before "present".

> Uh, the point is not that they need a magic block (though they do), but
> rather that the magic block is always checked.

It's a grammar complaint, not a factual complaint. Personally I'd
suggest wording it as "the magic block required in all ..."

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Josh Berkus wrote:
> >>> The <quote>magic block</> present in all PostgreSQL-supported libraries
> >>> is checked to guarantee compatibility. For this reason, non-PostgreSQL
> >>> libraries cannot be loaded in this way.
> >>
> >> you need "must be" before "present".
>
> > Uh, the point is not that they need a magic block (though they do), but
> > rather that the magic block is always checked.
>
> It's a grammar complaint, not a factual complaint. Personally I'd
> suggest wording it as "the magic block required in all ..."

OK, new text:

The <quote>magic block</> required in all PostgreSQL-supported libraries
is checked to guarantee compatibility. For this reason, non-PostgreSQL
libraries cannot be loaded in this way.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce,

> The <quote>magic block</> required in all PostgreSQL-supported libraries
> is checked to guarantee compatibility. For this reason, non-PostgreSQL
> libraries cannot be loaded in this way.

Um, that's identical to the old text. Try:

The <quote>magic block</> is required in all PostgreSQL-supported libraries
to guarantee compatibility.  For this reason, non-PostgreSQL libraries cannot
be loaded using preload_libraries.

--
Josh Berkus
PostgreSQL @ Sun
San Francisco

On Fri, Jun 16, 2006 at 12:01:53AM -0400, Bruce Momjian wrote:
> OK, new text:
>
> The <quote>magic block</> required in all PostgreSQL-supported libraries
> is checked to guarantee compatibility. For this reason, non-PostgreSQL
> libraries cannot be loaded in this way.

Heh. "guarantee"? We only check four things and it's possible to change
the system in all sorts of subtle ways (--integer-datetimes) that won't
be picked up. Ofcourse, for 99% of modules this is not a problem but
I'm not sure if the word "guarantee" is appropriate.

Maybe just "checked for compatibility", or "determine" or "ascertain".

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Fri, Jun 16, 2006 at 12:01:53AM -0400, Bruce Momjian wrote:
> > OK, new text:
> >
> > The <quote>magic block</> required in all PostgreSQL-supported libraries
> > is checked to guarantee compatibility. For this reason, non-PostgreSQL
> > libraries cannot be loaded in this way.
>
> Heh. "guarantee"? We only check four things and it's possible to change
> the system in all sorts of subtle ways (--integer-datetimes) that won't
> be picked up. Ofcourse, for 99% of modules this is not a problem but
> I'm not sure if the word "guarantee" is appropriate.
>
> Maybe just "checked for compatibility", or "determine" or "ascertain".

OK, new text in new paragraph:

Every PostgreSQL-supported library has a <quote>magic
block</> that is checked to guarantee compatibility.
For this reason, non-PostgreSQL libraries cannot be
loaded in this way.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Wed, 2006-06-14 at 20:07 -0400, Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > I just noticed (the hard way) that in 8.2CVS, the PG_MODULE_MAGIC header is
> > now *required* for all loadable modules. This includes non-pg modules,
> > such as Solaris' libumem (performance improvement for malloc).
>
> What is libumem and why are you trying to load it as a dynamic module?

http://www.usenix.org/event/usenix01/full_papers/bonwick/bonwick_html/index.html

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

On Fri, Jun 16, 2006 at 02:51:41PM +0100, Simon Riggs wrote:
> On Wed, 2006-06-14 at 20:07 -0400, Tom Lane wrote:
> > Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > > I just noticed (the hard way) that in 8.2CVS, the PG_MODULE_MAGIC header is
> > > now *required* for all loadable modules. This includes non-pg modules,
> > > such as Solaris' libumem (performance improvement for malloc).
> >
> > What is libumem and why are you trying to load it as a dynamic module?
>
> http://www.usenix.org/event/usenix01/full_papers/bonwick/bonwick_html/index.html

So it's a library that replaces malloc() and free() with new versions.
Does it actually help postgres, given postgres has its own memory
allocator already.

In any case, it would be fairly straightforward to make a duummy module
to wrap libumem. Create a file with just the module magic and link it
against libumem.

However, thinking about it, what they're doing can't possibly work. To
override malloc/free, you need to load the library *before* the C
library. Having the postmaster do it after startup is way too late.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> However, thinking about it, what they're doing can't possibly work. To
> override malloc/free, you need to load the library *before* the C
> library. Having the postmaster do it after startup is way too late.

Yeah, I was wondering about that too. At the very least you'd expect
some odd behaviors with memory malloc'd before the load vs memory
malloc'd after. Much more likely, though, it just fails to do anything
at all.

regards, tom lane