Is Backgroundworker.bgw_restart_time is defined in seconds?

Started by constzlover 4 years ago4 messagesbugs
Jump to latest
#1constzl
const_sunny@126.com

Hi,

typedef struct BackgroundWorker
{
char bgw_name[BGW_MAXLEN];
char bgw_type[BGW_MAXLEN];
int bgw_flags;
BgWorkerStartTime bgw_start_time;
int bgw_restart_time; /* in seconds, or BGW_NEVER_RESTART */
char bgw_library_name[BGW_MAXLEN];
char bgw_function_name[BGW_MAXLEN];
Datum bgw_main_arg;
char bgw_extra[BGW_EXTRALEN];
pid_t bgw_notify_pid; /* SIGUSR1 this backend on start/stop */
} BackgroundWorker;

static bool
SanityCheckBackgroundWorker(BackgroundWorker *worker, int elevel)
{
... ...

if ((worker->bgw_restart_time < 0 &&
worker->bgw_restart_time != BGW_NEVER_RESTART) ||
(worker->bgw_restart_time > USECS_PER_DAY / 1000))

The Backgroundworker.bgw_restart_time is defined in seconds,
so should it be "USECS_PER_DAY / 1000,000" here instead of "USECS_PER_DAY / 1000"?
Or am I getting it wrong?

{
ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("background worker \"%s\": invalid restart interval",
worker->bgw_name)));
return false;
}
... ...

}

Regards!

#2Daniel Gustafsson
daniel@yesql.se
In reply to: constzl (#1)
Re: Is Backgroundworker.bgw_restart_time is defined in seconds?

On 26 Aug 2021, at 09:38, constzl <const_sunny@126.com> wrote:

if ((worker->bgw_restart_time < 0 &&
worker->bgw_restart_time != BGW_NEVER_RESTART) ||
(worker->bgw_restart_time > USECS_PER_DAY / 1000))

The Backgroundworker.bgw_restart_time is defined in seconds,
so should it be "USECS_PER_DAY / 1000,000" here instead of "USECS_PER_DAY / 1000"?
Or am I getting it wrong?

bgw_restart_time is defined as "It can be any positive value” in the docs, see:

https://www.postgresql.org/docs/current/bgworker.html

My reading of the above code is that is tries to catch nonsensical values, not
to cap it to enforce restarts within a 24h period.

--
Daniel Gustafsson https://vmware.com/

#3constzl
const_sunny@126.com
In reply to: Daniel Gustafsson (#2)
Re:Re: Is Backgroundworker.bgw_restart_time is defined in seconds?

Yes, I got it wrong. You're right. Thank you for your explanation!<br/><br/>Best Regards,
At 2021-08-26 16:09:36, "Daniel Gustafsson" <daniel@yesql.se> wrote:

Show quoted text

On 26 Aug 2021, at 09:38, constzl <const_sunny@126.com> wrote:

if ((worker->bgw_restart_time < 0 &&
worker->bgw_restart_time != BGW_NEVER_RESTART) ||
(worker->bgw_restart_time > USECS_PER_DAY / 1000))

The Backgroundworker.bgw_restart_time is defined in seconds,
so should it be "USECS_PER_DAY / 1000,000" here instead of "USECS_PER_DAY / 1000"?
Or am I getting it wrong?

bgw_restart_time is defined as "It can be any positive value” in the docs, see:

https://www.postgresql.org/docs/current/bgworker.html

My reading of the above code is that is tries to catch nonsensical values, not
to cap it to enforce restarts within a 24h period.

--
Daniel Gustafsson https://vmware.com/

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#2)
Re: Is Backgroundworker.bgw_restart_time is defined in seconds?

Daniel Gustafsson <daniel@yesql.se> writes:

On 26 Aug 2021, at 09:38, constzl <const_sunny@126.com> wrote:
if ((worker->bgw_restart_time < 0 &&
worker->bgw_restart_time != BGW_NEVER_RESTART) ||
(worker->bgw_restart_time > USECS_PER_DAY / 1000))

My reading of the above code is that is tries to catch nonsensical values, not
to cap it to enforce restarts within a 24h period.

This may be intended to guard against integer overflows later (he guesses
without having read the code).

regards, tom lane