Обсуждение: 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 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
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 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 <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