Re: BackgroundPsql's set_query_timer_restart() may not work

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: BackgroundPsql's set_query_timer_restart() may not work
Date: 2023-11-29 08:19:11
Message-ID: CAD21AoBvu-KcjOFSCXG=AzKsZ3PymqK8iZnCVBgehBcgY1hgVA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 29, 2023 at 4:30 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Tue, Nov 28, 2023 at 12:23 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> writes:
> > > A nitpick on the patch - how about honoring the passed-in parameter
> > > with something like $self->{query_timer_restart} = 1 if !defined
> > > $self->{query_timer_restart}; instead of just setting it to 1 (a value
> > > other than undef) $self->{query_timer_restart} = 1;?
> >
> > I wondered about that too, but the evidence of existing callers is
> > that nobody cares. If we did make the code do something like that,
> > (a) I don't think your fragment is right, and (b) we'd need to rewrite
> > the function's comment to explain it. I'm not seeing a reason to
> > think it's worth spending effort on.

Agreed.

> Hm. I don't mind doing just the $self->{query_timer_restart} = 1; like
> in Sawada-san's patch.

Okay, I've attached the patch that I'm going to push through v16,
barring any objections.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v2-0001-Fix-set_query_timer_restart-didn-t-work-without-a.patch application/octet-stream 1.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2023-11-29 09:11:30 RE: Synchronizing slots from primary to standby
Previous Message Bharath Rupireddy 2023-11-29 07:30:32 Re: BackgroundPsql's set_query_timer_restart() may not work