Server is not getting started with log level as debug5 on master after commit 3147ac - Mailing list pgsql-hackers

From Amit Kapila
Subject Server is not getting started with log level as debug5 on master after commit 3147ac
Date
Msg-id CAA4eK1JR_vm=aGRbqfhtCkq1hQ3wWUjdVb1ZwHvy8kuzNp8U4w@mail.gmail.com
Whole thread Raw
Responses Re: Server is not getting started with log level as debug5 on master after commit 3147ac  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
In master branch, server is not getting started with log level as debug5.

Simple steps to reproduce the problem:
a. initdb -D ..\..\Data
b. change log_min_messages = debug5 in postgresql.conf
c. start server (pg_ctl start -D ..\..\Data)  -- server doesn't get started

Relevant message on server console:
DEBUG:  mapped win32 error code 2 to 2
FATAL:  could not open recovery command file "recovery.conf": No error

This problem occurs only in Windows, but the cause of problem is generic.

The problem occurs when during startup, code tries to open
recovery.conf in below function:
readRecoveryCommandFile(void)
{
..
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
{
if (errno == ENOENT)
    return; /* not there, so no archive recovery */
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not open recovery command file \"%s\": %m",
RECOVERY_COMMAND_FILE)));
..
}

It is expected that if file doesn't exist, errno should be ENOENT
which is correct and was working fine until commit:
3147acd63e0135aff9a6c4b01d861251925d97d9
Use improved vsnprintf calling logic in more places.

The reason is that after this commit, function appendStringInfoVA has
started calling pvsnprintf() which initializes errno to 0. As function
appendStringInfoVA gets called in error path
(errmsg_internal()->EVALUATE_MESSAGE->appendStringInfoVA), it can
reset previous errno if there is any, as it happens for function
_dosmaperr().

_dosmaperr(unsigned long e)
{
 ...
 errno = doserrors[i].doserr;
 #ifndef FRONTEND
 ereport(DEBUG5,
 (errmsg_internal("mapped win32 error code %lu to %d",
 e, errno)));
..
}

Here after setting errno, it calls errmsg_internal() which internally
resets errno to zero, so the callers of this function who are
expecting proper errno can fail which is what happens in current
issue.

I could think of below possible ways to fix the problem:
a. in function pvsnprintf(), save the errno before setting it to 0 and
then before exiting function reset it back to saved errno. I think
this is sane because in function pvsnprintf, if any error occurs due
to which errno is changed, it will not return control, so errno will
not be required by callers.
b. we can change the callers like _dosmaperr() who have responsibility
to save errno for their callers.

Patch with approach a) is attached with mail, we can change code as
per approach b) or any other better method as well, but for now I have
prepared patch with approach a), as I could not see any problem with
it.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: Fabrízio de Royes Mello
Date:
Subject: Re: psql should show disabled internal triggers
Next
From: Amit Kapila
Date:
Subject: Re: Add \i option to bring in the specified file as a quoted literal