On Fri, Jul 26, 2019 at 5:27 AM Michael Paquier <mich...@paquier.xyz> wrote: > > 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.
Sure, but there is no easy solution in that case, as you'd have to do all the work of spawning multiple reindexdb according to the underlying table, so probably what will happen here is that there'll just be two simple calls to reindexdb, one for the indexes, serialized anyway, and one for everything else. My vote is still to allow it, possibly emitting a notice or a warning. > > 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. Thanks for that! I'm fine with using switch to avoid future bad surprises. > 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. I see that you iterate over the SimpleStringList after it's generated. Why not computing that while building it in get_parallel_object_list (and keep the provided table list count) instead?
