From: Nathan Bossart <nathandbossart@gmail.com>
To: Euler Taveira <euler@eulerto.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: improve performance of pg_dump with many sequences
Date: Wed, 10 Jul 2024 17:05:11 -0500
Message-ID: <Zo8Fl53vBVS1Cnmt@nathan> (raw)
In-Reply-To: <0a0f4d52-ab75-4d69-abe8-5713bf70eee6@app.fastmail.com>
References: <20240503025140.GA1227404@nathanxps13>
<Zo2LdwUamOq6HzIU@nathan>
<0a0f4d52-ab75-4d69-abe8-5713bf70eee6@app.fastmail.com>
On Wed, Jul 10, 2024 at 05:08:56PM -0300, Euler Taveira wrote:
> Nice improvement. The numbers for a realistic scenario (10k sequences) are
Thanks for taking a look!
> You are changing internal representation from char to int64. Is the main goal to
> validate catalog data? What if there is a new sequence data type whose
> representation is not an integer?
IIRC 0001 was primarily intended to reduce the amount of memory needed for
the sorted table. Regarding a new sequence data type, I'm assuming we'll
have much bigger fish to fry if we do that (e.g., pg_sequence uses int8 for
the values), and I'd hope that adjusting this code wouldn't be too
difficult, anyway.
> This code path is adding zero byte to the last position of the fixed string. I
> suggest that the zero byte is added to the position after the string length.
I'm not following why that would be a better approach. strncpy() will add
a NUL to the end of the string unless it doesn't fit in the buffer, in
which case we'll add our own via "seqtype[sizeof(seqtype) - 1] = '\0'".
Furthermore, the compiler can determine the position where the NUL should
be placed, whereas placing it at the end of the copied string requires a
runtime strlen().
> l = strlen(PQgetvalue(res, 0, 0));
> Assert(l < sizeof(seqtype));
> strncpy(seqtype, PQgetvalue(res, 0, 0), l);
> seqtype[l] = '\0';
I think the strncpy() should really be limited to the size of the seqtype
buffer. IMHO an Assert is not sufficient.
> If you are not planning to apply 0003, make sure you fix collectSequences() to
> avoid versions less than 10. Move this part to 0002.
Yeah, no need to create the table if we aren't going to use it.
> Since you apply a fix for pg_sequence_last_value function, you can simplify the
> query in 0003. CASE is not required.
Unfortunately, I think we have to keep this workaround since older minor
releases of PostgreSQL don't have the fix.
> patched (0001 and 0002):
> real 0m0,290s
> user 0m0,038s
> sys 0m0,104s
>
> I'm not sure if 0003 is worth. Maybe if you have another table like you
> suggested.
What pg_dump command did you test here? Did you dump the sequence data, or
was this --schema-only?
--
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, euler@eulerto.com
Subject: Re: improve performance of pg_dump with many sequences
In-Reply-To: <Zo8Fl53vBVS1Cnmt@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