Re: Add parallelism and glibc dependent only options to reindexdb

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

In response to

Responses

Browse pgsql-hackers by date

  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)