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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: 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 02:43:58
Message-ID: CALj2ACXvMG6sgkJduOEUicLvJL5=pcOKEPusuYPZAjiCA1oKAw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Regards,
Bharath Rupireddy.

Attachment Content-Type Size
v1-0001-autovacuum-be-specific-about-error-message-and-me.patch application/octet-stream 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-10-28 02:45:24 Re: pgsql: Document XLOG_INCLUDE_XID a little better
Previous Message Amit Kapila 2021-10-28 02:41:58 Re: Added schema level support for publication.