Re: Isn't it better with "autovacuum worker...." instead of "worker took too long to start; canceled" specific to "auto

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Isn't it better with "autovacuum worker...." instead of "worker took too long to start; canceled" specific to "auto
Date: 2021-10-28 07:11:16
Message-ID: CAD21AoBAM-nY_YTqZSQhvQfvFX2CpWRY=jYXWbNURuZSOPKGHQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 28, 2021 at 11:44 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Thu, Oct 28, 2021 at 7:11 AM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > At Wed, 27 Oct 2021 14:26:11 -0500, Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote in
> > > On Wed, Oct 27, 2021 at 07:05:10PM +0000, Bossart, Nathan wrote:
> > > > My vote is to just change it to
> > > >
> > > > ereport(WARNING,
> > > > (errmsg("autovacuum worker took too long to start; canceled")));
> > > >
> > > > and call it a day. If we wanted to add errdetail(), I think we should
> > > > make sure it is providing useful context, but I'm not sure what that
> > > > might look like.
> > >
> > > I think that's fine.
> >
> > +1
>
> Done.
>
> > > Note that the backend_type is illuminating for those who use CSV logs, or use
> > > P13+ and log_line_prefix += %b (see 70a7b4776).
> > >
> > > session_line | 1
> > > error_severity | WARNING
> > > message | worker took too long to start; canceled
> > > backend_type | autovacuum launcher
> >
> > Yeah, the additional "autovacuum" is not noisy at all even in that
> > context. Some other messages are prefixed with "autovacuum".
> >
> > "could not fork autovacuum worker process: %m"
> > "autovacuum worker started without a worker entry"
> >
> > By a quick look all occurances of "laucher" are prefixed with
> > "autovacuum" or "logical replcaion", which seems fine.
> >
> > As a related topic, autovacuum.c has another use of bare "worker"s.
> >
> > > tmpcxt = AllocSetContextCreate(CurrentMemoryContext,
> > > "Start worker tmp cxt",
> > > ALLOCSET_DEFAULT_SIZES);
> >
> > > AutovacMemCxt = AllocSetContextCreate(TopMemoryContext,
> > > "AV worker",
> > > ALLOCSET_DEFAULT_SIZES);
> >
> > I'm not sure the former needs to be fixed, but the latter is actually
> > visible to users via pg_log_backend_memory_contexts().
> >
> > LOG: level: 1; AV worker: 8192 total in 1 blocks; 7928 free (0 chunks); 264 used
>
> Good catch. I've seen the use of "AV" in some of the mem context
> names, why that? Let's be specific and say "Autovacuum". Attached
> patch does that. Please review it.

+1. The patch looks good to me too.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Rijkers 2021-10-28 07:29:10 Re: [PATCH] Proposal for HIDDEN/INVISIBLE column
Previous Message Kyotaro Horiguchi 2021-10-28 06:57:51 Re: inefficient loop in StandbyReleaseLockList()