Re: Finding database for pg_upgrade missing library

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Finding database for pg_upgrade missing library
Date: 2018-07-24 08:33:48
Message-ID: 52B7235E-0557-4A4C-A2D4-434871B8DF51@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 16 Jul 2018, at 14:13, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Sat, Jul 14, 2018 at 12:14:51AM +0200, Daniel Gustafsson wrote:
>>> On 13 Jul 2018, at 18:28, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>>
>>> I received a private pg_upgrade feature request to report the database
>>> name for missing loadable libraries. Currently we report "could not
>>> load library" and the library file name, e.g. $libdir/pgpool-regclass.
>>>
>>> The request is that we report the _database_ name that contained the
>>> loadable library reference. However, that isn't easy to do because we
>>> gather all loadable library file names, sort them, and remove
>>> duplicates, for reasons of efficiency and so we check libraries in a
>>> predictable alphabetical order.
>>>
>>> Is it worth modifying pg_upgrade to report the first or all databases
>>> that contain references to missing loadable libraries? I don't think
>>> so, but I wanted to ask here.
>>
>> I ran into that very problem when upgrading 50+ clusters during a long night a
>> while back, and patched pg_upgrade to report the database which helped a lot
>> (or at least helped me be a bit lazy). So, +1 on the feature from me. If
>> there is interest I can see if I can dig out the patch and polish it up.
>
> Yes, please post the patch. Seems we now have three people who want
> this. Even though it is related to reporting errors, I think this is a
> new feature so will only be in PG 12.

Completely agree.

Sorry for the delay in posting the this. I dug out the code I used when
upgrading my clusters, rebased and adapted the patch slightly and it seems to
work as expected.

> Looking at the code, we do a qsort(), then detect (since they are all
> now adjacent) and remove the duplicate references to the library. What
> I think should be done is to move the duplicate detection down to the
> place where we check for the library, then print all the database names
> of the duplicates if we don't find the library. I guess we either need
> a separate array for the database name, or a 'struct' that holds the
> library name and database name.

I went for the solution of not removing the duplicates in the first pass, and
instead dedup during reporting. It’s sort of unelegant in that it consumes
more memory than strictly limited, but the simplicity of the code weighs up
IMO. An additional step to free the libraries struct member before continuing
could perhaps be worth it?

cheers ./daniel

Attachment Content-Type Size
pg_upgrade_loadlib_db.patch application/octet-stream 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-07-24 08:50:09 Re: [report] memory leaks in COPY FROM on partitioned table
Previous Message Sergei Kornilov 2018-07-24 08:19:51 Re: [FEATURE PATCH] pg_stat_statements with plans (v02)