pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Is Backgroundworker.bgw_restart_time is defined in seconds?
4+ messages / 3 participants
[nested] [flat]

* Is Backgroundworker.bgw_restart_time is defined in seconds?
@ 2021-08-26 07:38  constzl <const_sunny@126.com>
  0 siblings, 1 reply; 4+ messages in thread

From: constzl @ 2021-08-26 07:38 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org



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!

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Is Backgroundworker.bgw_restart_time is defined in seconds?
@ 2021-08-26 08:09  Daniel Gustafsson <daniel@yesql.se>
  parent: constzl <const_sunny@126.com>
  0 siblings, 2 replies; 4+ messages in thread

From: Daniel Gustafsson @ 2021-08-26 08:09 UTC (permalink / raw)
  To: constzl <const_sunny@126.com>; +Cc: pgsql-bugs@lists.postgresql.org

> 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/






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re:Re: Is Backgroundworker.bgw_restart_time is defined in seconds?
@ 2021-08-26 08:36  constzl <const_sunny@126.com>
  parent: Daniel Gustafsson <daniel@yesql.se>
  1 sibling, 0 replies; 4+ messages in thread

From: constzl @ 2021-08-26 08:36 UTC (permalink / raw)
  To: Daniel Gustafsson <daniel@yesql.se>; +Cc: pgsql-bugs@lists.postgresql.org

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:
>> 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/
>
>


^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Is Backgroundworker.bgw_restart_time is defined in seconds?
@ 2021-08-26 14:07  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Daniel Gustafsson <daniel@yesql.se>
  1 sibling, 0 replies; 4+ messages in thread

From: Tom Lane @ 2021-08-26 14:07 UTC (permalink / raw)
  To: Daniel Gustafsson <daniel@yesql.se>; +Cc: constzl <const_sunny@126.com>; pgsql-bugs@lists.postgresql.org

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





^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2021-08-26 14:07 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-08-26 07:38 Is Backgroundworker.bgw_restart_time is defined in seconds? constzl <const_sunny@126.com>
2021-08-26 08:09 ` Daniel Gustafsson <daniel@yesql.se>
2021-08-26 08:36   ` constzl <const_sunny@126.com>
2021-08-26 14:07   ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox