On Mon, Aug 26, 2019 at 02:17:06AM +0000, Michael Paquier wrote: > Fix error handling of vacuumdb and reindexdb when running out of fds > > When trying to use a high number of jobs, vacuumdb (and more recently > reindexdb) has only checked for a maximum number of jobs used, causing > confusing failures when running out of file descriptors when the jobs > open connections to Postgres. This commit changes the error handling so > as we do not check anymore for a maximum number of allowed jobs when > parsing the option value with FD_SETSIZE, but check instead if a file > descriptor is within the supported range when opening the connections > for the jobs so as this is detected at the earliest time possible. > > Also, improve the error message to give a hint about the number of jobs > recommended, using a wording given by the reviewers of the patch.
jacana does not like that, and has reported an error for 9.6: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2019-08-26 03%3A51%3A19 # Running: vacuumdb -zj2 postgres vacuumdb: vacuuming database "postgres" vacuumdb: too many jobs for this platform -- try 1 I am able to reproduce the problem locally, and the problem is that we don't declare FD_SETSIZE on Windows before winsock2.h in scripts_parallel.c (or vacuumdb.c in ~12) so all the Windows machines running TAP are going to complain on that. This matches with the same problem reported here https://www.postgresql.org/message-id/1248903114.6348.195.camel@lapdragon We have never done that before in vacuumdb.c, and because of that I think that with a high number of jobs we run into buffer overflows. The patch attached does the job on my end, any objections? There is an argument to do that in win32_port.h, but for now I'd rather take a safe path, or just do for the change in win32_port.h on HEAD. (Noticed the missing newline as well in the error string in back-branches... I'll address it at the same time.) -- Michael
diff --git a/src/bin/scripts/scripts_parallel.c b/src/bin/scripts/scripts_parallel.c index 55bda9044b..97435160e9 100644 --- a/src/bin/scripts/scripts_parallel.c +++ b/src/bin/scripts/scripts_parallel.c @@ -12,6 +12,10 @@ *------------------------------------------------------------------------- */ +#ifdef WIN32 +#define FD_SETSIZE 1024 /* must set before winsock2.h is included */ +#endif + #include "postgres_fe.h" #ifdef HAVE_SYS_SELECT_H
signature.asc
Description: PGP signature
