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
