On Mon, 27 Jan 2020 at 14:38, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Sun, Jan 26, 2020 at 12:29:38PM -0800, Andres Freund wrote:
> > > postgres=# SET client_min_messages=debug;SET statement_timeout=99; VACUUM (VERBOSE, PARALLEL 0) t;
> > > INFO: vacuuming "public.t"
> > > DEBUG: "t_a_idx": vacuuming index
> > > 2020-01-20 15:47:36.338 CST [20139] ERROR: canceling statement due to statement timeout
> > > 2020-01-20 15:47:36.338 CST [20139] CONTEXT: while vacuuming relation "public.t_a_idx"
> > > 2020-01-20 15:47:36.338 CST [20139] STATEMENT: VACUUM (VERBOSE, PARALLEL 0) t;
> > > ERROR: canceling statement due to statement timeout
> > > CONTEXT: while vacuuming relation "public.t_a_idx"
> >
> > It'd be a bit nicer if it said index "public.t_a_idx" for relation "public.t".
>
> I think that tips the scale in favour of making vacrelstats a global.
> I added that as a 1st patch, and squished the callback patches into one.
Hmm I don't think it's a good idea to make vacrelstats global. If we
want to display the relation name and its index name in error context
we might want to define a new struct dedicated for error context
reporting. That is it has blkno, stage and relation name and schema
name for both table and index and then we set these variables of
callback argument before performing a vacuum phase. We don't change
LVRelStats at all.
Although the patch replaces get_namespace_name and
RelationGetRelationName but we use namespace name of relation at only
two places and almost ereport/elog messages use only relation name
gotten by RelationGetRelationName which is a macro to access the
relation name in Relation struct. So I think adding relname to
LVRelStats would not be a big benefit. Similarly, adding table
namespace to LVRelStats would be good to avoid calling
get_namespace_name whereas I'm not sure it's worth to have it because
it's expected not to be really many times.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services