Re: Minor version upgrades and extension packaging

[Alvaro Herrera]

Andres Freund wrote:

> Ugh. I personally would say that's because that commit did stuff that we
> normally trie hard not to do. While ColumnDef at least isn't serialized
> into catalogs, we normally trie hard to break struct layout. Peter,
> shouldn't that field at the very least have been added at the end?

FWIW we just noticed that because commit 699bf7d05 changed the
heap_prepare_freeze_tuple() ABI back in December, it caused similar
breakage :-(  I didn't notice the implications when I read the patch.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Minor version upgrades and extension packaging

[David Fetter]

On Mon, Feb 12, 2018 at 01:04:40PM -0500, Mat Arye wrote:
> On Mon, Feb 12, 2018 at 8:41 AM, Peter Eisentraut <
> peter.eisentr...@2ndquadrant.com> wrote:
> 
> > On 2/11/18 23:14, Andres Freund wrote:
> > > On 2018-02-11 22:19:30 -0500, Tom Lane wrote:
> > >> Not sure what to do about it at this point.  We could move that field to
> > >> the end for 10.3, leaving 10.2 as the only ABI-incompatible minor
> > release,
> > >> but I don't know that that really makes things any better than leaving
> > it
> > >> as-is.  Somewhere around the dot-two minor release is where uptake of a
> > >> new PG branch starts to become significant, IME, so preserving ABI
> > >> compatibility going forward from 10.2 might be more useful than
> > preserving
> > >> it against 10.0/10.1.
> > >
> > > Yea, I think the damage is done in this case, and we shouldn't make
> > > things even more complicated.
> >
> > Yeah, lesson learned.  Sorry.
> >
> >
> Thanks all for your responses. FWIW I agree that it's best to not revert
> this struct change and just keep ABI compatibility with 10.2 going forward.
> We will also integrate testing against the tip of 10 nightly going forward
> so that we can hopefully catch and report such issues early.

If it's not too much trouble, you could add whatever you're doing as
buildfarm module(s), as illustrated with BlackholeFDW.pm,
FileTextArrayFDW.pm, and RedisFDW.pm here:

https://github.com/PGBuildFarm/client-code/tree/master/PGBuild/Modules

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Minor version upgrades and extension packaging

[Mat Arye]

On Mon, Feb 12, 2018 at 8:41 AM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 2/11/18 23:14, Andres Freund wrote:
> > On 2018-02-11 22:19:30 -0500, Tom Lane wrote:
> >> Not sure what to do about it at this point.  We could move that field to
> >> the end for 10.3, leaving 10.2 as the only ABI-incompatible minor
> release,
> >> but I don't know that that really makes things any better than leaving
> it
> >> as-is.  Somewhere around the dot-two minor release is where uptake of a
> >> new PG branch starts to become significant, IME, so preserving ABI
> >> compatibility going forward from 10.2 might be more useful than
> preserving
> >> it against 10.0/10.1.
> >
> > Yea, I think the damage is done in this case, and we shouldn't make
> > things even more complicated.
>
> Yeah, lesson learned.  Sorry.
>
>
Thanks all for your responses. FWIW I agree that it's best to not revert
this struct change and just keep ABI compatibility with 10.2 going forward.
We will also integrate testing against the tip of 10 nightly going forward
so that we can hopefully catch and report such issues early.

Thanks again,
Mat

Re: Minor version upgrades and extension packaging

[Peter Eisentraut]

On 2/11/18 23:14, Andres Freund wrote:
> On 2018-02-11 22:19:30 -0500, Tom Lane wrote:
>> Not sure what to do about it at this point.  We could move that field to
>> the end for 10.3, leaving 10.2 as the only ABI-incompatible minor release,
>> but I don't know that that really makes things any better than leaving it
>> as-is.  Somewhere around the dot-two minor release is where uptake of a
>> new PG branch starts to become significant, IME, so preserving ABI
>> compatibility going forward from 10.2 might be more useful than preserving
>> it against 10.0/10.1.
> 
> Yea, I think the damage is done in this case, and we shouldn't make
> things even more complicated.

Yeah, lesson learned.  Sorry.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Minor version upgrades and extension packaging

[Andres Freund]

On 2018-02-11 22:19:30 -0500, Tom Lane wrote:
> Not sure what to do about it at this point.  We could move that field to
> the end for 10.3, leaving 10.2 as the only ABI-incompatible minor release,
> but I don't know that that really makes things any better than leaving it
> as-is.  Somewhere around the dot-two minor release is where uptake of a
> new PG branch starts to become significant, IME, so preserving ABI
> compatibility going forward from 10.2 might be more useful than preserving
> it against 10.0/10.1.

Yea, I think the damage is done in this case, and we shouldn't make
things even more complicated.

Greetings,

Andres Freund

Re: Minor version upgrades and extension packaging

[Tom Lane]

Mat Arye  writes:
> We recently found that people who had compiled the TimescaleDB extension
> against 10.1 (or installed our binary versions via yum, apt, etc.) had
> their extension break when they upgraded to 10.2 due to changes of some
> underlying structs between the two minor versions.
> In particular, in the commit
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1597948c962a1407c01fc492c44917c097efa92e
> the structure of the ColumnDef struct changed.

BTW, while there was surely not a huge amount of daylight between that
commit on 2 Feb and the 10.2 wrap on 5 Feb, there was enough time to have
fixed the problem given prompt feedback.  I suggest that Timescale would
be well advised to set up a buildfarm animal that has an additional module
to run basic binary-compatibility testing against your extension in the
back branches.  I don't know a lot about writing additional test modules
for the buildfarm script, but it's definitely possible.  Andrew Dunstan
might be able to offer more specific advice.

regards, tom lane

Re: Minor version upgrades and extension packaging

[Tom Lane]

Andres Freund  writes:
> On 2018-02-11 21:50:32 -0500, Mat Arye wrote:
>> In particular, in the commit
>> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1597948c962a1407c01fc492c44917c097efa92e
>> the structure of the ColumnDef struct changed.

> Ugh. I personally would say that's because that commit did stuff that we
> normally trie hard not to do. While ColumnDef at least isn't serialized
> into catalogs, we normally trie hard to break struct layout. Peter,
> shouldn't that field at the very least have been added at the end?

Yeah.  The position of the field makes sense for HEAD, but it would have
been good practice to add it at the end in released branches.  That's
what we normally do when we have to make struct changes in back branches.
That isn't a 100% fix for ABI compatibility problems --- if you're making
new instances of the node type in an extension, you still lose --- but
it avoids problems in many cases.

Not sure what to do about it at this point.  We could move that field to
the end for 10.3, leaving 10.2 as the only ABI-incompatible minor release,
but I don't know that that really makes things any better than leaving it
as-is.  Somewhere around the dot-two minor release is where uptake of a
new PG branch starts to become significant, IME, so preserving ABI
compatibility going forward from 10.2 might be more useful than preserving
it against 10.0/10.1.

regards, tom lane

Re: Minor version upgrades and extension packaging

[Andres Freund]

Hi,


On 2018-02-11 21:50:32 -0500, Mat Arye wrote:
> We recently found that people who had compiled the TimescaleDB extension
> against 10.1 (or installed our binary versions via yum, apt, etc.) had
> their extension break when they upgraded to 10.2 due to changes of some
> underlying structs between the two minor versions.
> 
> In particular, in the commit
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1597948c962a1407c01fc492c44917c097efa92e
> the structure of the ColumnDef struct changed. Since TimescaleDB uses an
> event hook that makes use of the ColumnDef structure, the TimescaleDB
> shared library compiled under 10.1 expected a different struct packing for
> ColumnDef than what was available in 10.2.

Ugh. I personally would say that's because that commit did stuff that we
normally trie hard not to do. While ColumnDef at least isn't serialized
into catalogs, we normally trie hard to break struct layout. Peter,
shouldn't that field at the very least have been added at the end?

> I had three questions:
> 
> 1) Are similar changes to struct packing expected under minor postgres
> releases (we haven't encountered this issue before)?

Normally not.


> 2) Is it expected to have to recompile extensions for minor version
> upgrades of Postgres?

Normally only in extraordinary cases. There e.g. had been security
issues that required changes in PL handlers.

Greetings,

Andres Freund