Minor version upgrades and extension packaging
Hi All,
I am writing to get some advice on extension packaging for minor version
upgrades in Postgres.
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.
I had three questions:
1) Are similar changes to struct packing expected under minor postgres
releases (we haven't encountered this issue before)?
2) Is it expected to have to recompile extensions for minor version
upgrades of Postgres?
3) How do other extensions deal with this issue?
Any other suggestions to handle these types of issues would be
appreciated. One obvious solution with binary releases is to have one for
every minor version, although this leads to both release and deployment
complexity.
Thanks,
Mat
TimescaleDB
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
Andres Freund <andres@anarazel.de> 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
Mat Arye <mat@timescale.com> 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
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
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
On Mon, Feb 12, 2018 at 8:41 AM, Peter Eisentraut <
peter.eisentraut@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 minorrelease,
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 thanpreserving
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
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.eisentraut@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 minorrelease,
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 thanpreserving
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 <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services