From: Nathan Bossart <nathandbossart@gmail.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: optimizing pg_upgrade's once-in-each-database steps
Date: Fri, 19 Jul 2024 16:21:37 -0500
Message-ID: <ZprY4e23vR95Nc51@nathan> (raw)
In-Reply-To: <4493D323-9726-48F7-A702-E71FB991F76B@yesql.se>
References: <3b97ab8e295ec3db22baa2dfdd49073449ae1696.camel@j-davis.com>
<20240517012408.GA1691322@nathanxps13>
<ZoMHsBasBy8LqkEU@nathan>
<CA+TgmoaBof36-ZuRWtu=+8rnhGTNQFFGNcmVB0x3aQawfc_JxA@mail.gmail.com>
<ZoMK9ZhOsWEKqxlM@nathan>
<ZowEIGd0lLKh16qe@nathan>
<ZoyveaS6a6jyhYS6@nathan>
<26742670-9539-4F30-B6FA-9C1DB743BC23@yesql.se>
<Zpg4WnS9BRWbgBLi@nathan>
<4493D323-9726-48F7-A702-E71FB991F76B@yesql.se>
On Thu, Jul 18, 2024 at 09:57:23AM +0200, Daniel Gustafsson wrote:
>> On 17 Jul 2024, at 23:32, Nathan Bossart <nathandbossart@gmail.com> wrote:
>> On Wed, Jul 17, 2024 at 11:16:59PM +0200, Daniel Gustafsson wrote:
>
>>> +static void
>>> +dispatch_query(const ClusterInfo *cluster, AsyncSlot *slot,
>>> ....
>>> + pg_free(query);
>>> +}
>>>
>>> A minor point, perhaps fueled by me not having played around much with this
>>> patchset. It seems a bit odd that dispatch_query is responsible for freeing
>>> the query from the get_query callback. I would have expected the output from
>>> AsyncTaskGetQueryCB to be stored in AsyncTask and released by async_task_free.
>>
>> I don't see any problem with doing it the way you suggest.
Actually, I do see a problem. If we do it this way, we'll have to store a
string per database somewhere, which seems unnecessary.
However, while looking into this, I noticed that only one get_query
callback (get_db_subscription_count()) actually customizes the generated
query using information in the provided DbInfo. AFAICT we can do this
particular step without running a query in each database, as I mentioned
elsewhere [0]. That should speed things up a bit and allow us to simplify
the AsyncTask code.
With that, if we are willing to assume that a given get_query callback will
generate the same string for all databases (and I think we should), we can
run the callback once and save the string in the step for dispatch_query()
to use. This would look more like what you suggested in the quoted text.
[0] https://postgr.es/m/ZprQJv_TxccN3tkr%40nathan
--
nathan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, daniel@yesql.se, robertmhaas@gmail.com, pgsql@j-davis.com
Subject: Re: optimizing pg_upgrade's once-in-each-database steps
In-Reply-To: <ZprY4e23vR95Nc51@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox