From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Julien Rouhaud <julien(dot)rouhaud(at)free(dot)fr> |
Cc: | Sergei Kornilov <sk(at)zsrv(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Add parallelism and glibc dependent only options to reindexdb |
Date: | 2019-07-26 03:27:06 |
Message-ID: | 20190726032706.GE7677@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jul 25, 2019 at 01:00:34PM +0200, Julien Rouhaud wrote:
> The problem is that a user doing something like:
>
> reindexdb -j48 -i some_index -S s1 -S s2 -S s3....
>
> will probably be disappointed to learn that he has to run a specific
> command for the index(es) that should be reindexed. Maybe we can
> issue a warning that parallelism isn't used when an index list is
> processed and user asked for multiple jobs?
Arguments go in both directions as some other users may be surprised
by the performance of indexes as serialization is enforced.
> I don't send a new patch since the --index wanted behavior is not
> clear yet.
So I am sending one patch (actually two) after a closer review that I
have spent time shaping into a committable state. And for this part I
have another suggestion that is to use a switch/case without a default
so as any newly-added object types would allow somebody to think about
those code paths as this would generate compiler warnings.
While reviewing I have found an extra bug in the logic: when using a
list of tables, the number of parallel slots is the minimum between
concurrentCons and tbl_count, but this does not get applied after
building a list of tables for a schema or database reindex, so we had
better recompute the number of items in reindex_one_database() before
allocating the number of parallel slots. There was also a small gem
in the TAP tests for one of the commands using "-j2" in one of the
command arguments.
So here we go:
- 0001 is your original thing, with --jobs enforced to 1 for the index
part.
- 0002 is my addition to forbid --index with --jobs.
I am fine to be outvoted regarding 0002, and it is the case based on
the state of this thread with 2:1. We could always revisit this
decision in this development cycle anyway.
--
Michael
Attachment | Content-Type | Size |
---|---|---|
0001-Base-patch-for-support-of-jobs-in-reindexdb.patch | text/x-diff | 20.4 KB |
0002-Forbid-index-with-jobs.patch | text/x-diff | 3.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2019-07-26 03:30:55 | Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS) |
Previous Message | Alvaro Herrera | 2019-07-26 02:57:08 | Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS) |