Re: EDB builds Postgres 13 with an obsolete ICU version

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Daniel Verite <daniel(at)manitou-mail(dot)org>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Sandeep Thakkar <sandeep(dot)thakkar(at)enterprisedb(dot)com>
Subject: Re: EDB builds Postgres 13 with an obsolete ICU version
Date: 2020-08-17 15:55:13
Message-ID: CA+OCxowimXSQbrVhCLaa+f-XV50m0bwFhxp6Z7i2onUSBsWhmg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 17, 2020 at 4:14 PM Magnus Hagander <magnus(at)hagander(dot)net> wrote:

>
>
> On Mon, Aug 17, 2020 at 1:44 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Mon, Aug 17, 2020 at 11:19 AM Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>>
>>>
>>>
>>> On Fri, Aug 14, 2020 at 3:00 PM Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>>
>>>> On Tue, Aug 11, 2020 at 02:58:30PM +0200, Magnus Hagander wrote:
>>>> > On Tue, Aug 4, 2020 at 11:42 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>> > That would require fairly large changes to the installer to allow
>>>> it to
>>>> > login to the database server (whether that would work would
>>>> be dependent on
>>>> > how pg_hba.conf is configured), and also assumes that the ICU ABI
>>>> hasn't
>>>> > changed between releases. It would also require some hacky
>>>> renaming of
>>>> > DLLs, as they have the version number in them.
>>>> >
>>>> > I assumed it had code for that stuff already. Mainly because I
>>>> assumed it
>>>> > supported doing pg_upgrade, which requires similar things no?
>>>>
>>>
>> No, the installers don't support pg_upgrade directly. They ship it of
>> course, and the user can manually run it, but the installers won't do that,
>> and have no ability to login to a cluster except during the post-initdb
>> phase.
>>
>
> Oh, I just assumed it did :)
>
> If it doesn't, I think shipping with a modern ICU is a much smaller
> problem really...
>
>
> While pg_upgrade requires having the old and new cluster software in
>>>> place, I don't think it helps allowing different ICU versions for each
>>>> cluster.
>>>
>>>
>>> Depends on where they are installed (and disclaimer, I don't know how
>>> the windows installers do that). But as long as the ICU libraries are
>>> installed in separate locations for the two versions, which I *think* they
>>> are or at least used to be, it shouldn't have an effect on this in either
>>> direction.
>>>
>>
>> They are.
>>
>
> Good. So putting both in wouldn't break things.
>
>
>
> That argument really only holds for different versions, not for different
>>> clusters of the same version. But I don't think the installers (natively)
>>> supports multiple clusters of the same version anyway.
>>>
>>
>> They don't. You'd need to manually init a new cluster and register a new
>> server instance. The installer only has any knowledge of the cluster it
>> sets up.
>>
>
> I'd say that's "unsupported enough" to not be a scenario one has to
> consider.
>

Agreed. Plus it's not really any different from running multiple clusters
on other OSs where we're likely to be using a vendor supplied ICU that the
user also couldn't change easily.

>
>
>
>>> The tricky thing is if you want to allow the user to *choose* which ICU
>>> version should be used with postgres version <x>. Because then the user
>>> might also expect an upgrade-path wherein they only upgrade the icu library
>>> on an existing install...
>>>
>>>
>>>> I guess you can argue that if you know the user is _not_ going
>>>> to be using pg_upgrade, then a new ICU version should be used for the
>>>> new cluster.
>>>>
>>>
>>> Yes, that's exactly the argument I meant :) If the user got the option
>>> to "pick version of ICU: <old>, <new>", with a comment saying "pick old
>>> only if you plan to do a pg_upgrade based upgrade of a different cluster,
>>> or if this instance should participate in replication with a node using
>>> <old>", that would probably help for the vast majority of cases. And of
>>> course, if the installer through other options can determine with certainty
>>> that it's going to be running pg_upgrade for the user, then it can reword
>>> the dialog based on that (that is, it should still allow the user to pick
>>> the new version, as long as they know that their indexes are going to need
>>> reindexing)
>>>
>>
>> That seems like a very hacky and extremely user-unfriendly approach. How
>> many users are going to understand options in the installer to deal with
>> that, or want to go decode the ICU filenames on their existing
>> installations (which our installers may not actually know about) to figure
>> out what their current version is?
>>
>
>
> That was more if the installer actually handles the whole chain. It
> clearly doesn't today (since it doesn't support upgrades), I agree this
> might definitely be overkill. But then also I don't really see the problem
> with just putting a new version of ICU in with the newer versions of
> PostgreSQL. That's just puts the user in the same position as they are with
> any other platform wrt manual pg_upgrade runs.
>

Well we can certainly do that if everyone is happy in the knowledge that
it'll mean pg_upgrade users will need to reindex if they've used ICU
collations.

Sandeep; can you have someone do a test build with the latest ICU please
(for background, this would be with the Windows and Mac installers)? If
noone objects, we can push that into the v13 builds before GA. We'd also
need to update the README if we do so.

>
>
>
>>
>> I would suggest that the better way to handle this would be for
>> pg_upgrade to (somehow) check the ICU version on the old and new clusters
>> and if there's a mismatch perform a reindex of any ICU based indexes. I
>> suspect that may require that the server exposes the ICU version though.
>> That way, the installers could freely upgrade the ICU version with a new
>> major release.
>>
>
> Having pg_upgrade spit out a script that does reindex specifically on the
> indexes required would certainly be useful in the generic case, and help
> others as well.
>

+1

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Borisov 2020-08-17 16:04:32 Re: [PATCH] Covering SPGiST index
Previous Message Drouvot, Bertrand 2020-08-17 15:47:13 Re: Add information to rm_redo_error_callback()