The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: tested, passed
Hi
I did some review and have few notes about behavior.
reindex database does not work with concurrently option:
> ./inst/bin/reindexdb --echo -d postgres -j 8 --concurrently
> SELECT pg_catalog.set_config('search_path', '', false);
> REINDEX SYSTEM CONCURRENTLY postgres;
> reindexdb: error: reindexing of system catalogs on database "postgres" failed: ERROR: cannot reindex system catalogs
concurrently
I think we need print message and skip system catalogs for concurrently reindex.
Or we can disallow concurrently database reindex with multiple jobs. I prefer first option.
> + reindex_one_database(dbname, REINDEX_SCHEMA, &schemas, host,
> + port, username, prompt_password, progname,
> + echo, verbose, concurrently,
> + Min(concurrentCons, nsp_count));
Should be just concurrentCons instead of Min(concurrentCons, nsp_count)
reindex_one_database for REINDEX_SCHEMA will build tables list and then processing by available workers. So:
-j 8 -S public -S public -S public -S poblic -S public -S public - will work with 6 jobs (and without multiple
processingfor same table)
-j 8 -S public - will have only one worker regardless tables count
> if (concurrentCons > FD_SETSIZE - 1)
"if (concurrentCons >= FD_SETSIZE)" would not cleaner? Well, pgbench uses >= condition, vacuumdb uses > FD_SETSIZE - 1.
Nomore FD_SETSIZE in conditions =)
regards, Sergei
