Re: pgsql: Fix error handling of vacuumdb and reindexdb when running out of

On 8/26/19 1:40 AM, Michael Paquier wrote:
> 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.)

Looks OK.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services