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 processing for 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. No more FD_SETSIZE in conditions =)
regards, Sergei
