Re: %d in log_line_prefix doesn't work for bg/autovacuum workers

Поиск
Список
Период
Сортировка
От Andres Freund
Тема Re: %d in log_line_prefix doesn't work for bg/autovacuum workers
Дата
Msg-id 20140516221746.GB9100@awork2.anarazel.de
обсуждение исходный текст
Ответ на Re: %d in log_line_prefix doesn't work for bg/autovacuum workers  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: %d in log_line_prefix doesn't work for bg/autovacuum workers
Список pgsql-hackers
On 2014-05-16 17:48:28 -0400, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> >> It's imo pretty annoying that neither bgworkers nor autovacuum workers
> >> show the proper database in the log. Why don't we just populate a global
> >> variable in InitPostgres() once we're sure which database the backend is
> >> connected to? We could fill fake MyProcPorts, but that doesn't seem like
> >> a good idea to me.
> 
> > The attached simple patch implements the former idea.
> 
> This is basically reintroducing a variable that used to exist and was
> intentionally gotten rid of, on the grounds that it'd fail to track
> any renaming of the session's current database (cf commit b256f24264).
> I'm not sure how much effort ought to be put into dealing with that corner
> case; but let's not reintroduce old bugs just for failure to remember
> them.

I did look whether there's a race making MyDatabaseName out of date. I
didn't see any. There's a windows where it could be renamed between
where I'd done the MyDatabaseName = ... and the LockSharedObject() a few
lines below. But directly afterwards there's a check that the database name is
still correct.
We could move the filling of MyDatabaseName to lessen the amount of
comments that need to be added.

It looks like the interlock around database names didn't exist back in
b256f24264 - so the situation simply has changed. Looks like that has
happend in 2667d56a3b489e5645f0.

> The existing implementation of log line %d has the same issue of course,
> so this is not a very good argument not to change %d.  It *is* a reason
> not to blindly change get_database_name(MyDatabaseId) calls, which were
> coded that way precisely for this reason.  It might also be a reason
> not to blithely expose a global variable like this, which would encourage
> making of the same mistake in places that might be more critical than %d.

I think exposing the variable somewhat reasonable. It allows to to print
the database name in situations where we'd normally not dare because of
the syscache lookup... And given that it doesn't look racy anymore the
danger seems pretty low.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: %d in log_line_prefix doesn't work for bg/autovacuum workers
Следующее
От: Andres Freund
Дата:
Сообщение: Re: %d in log_line_prefix doesn't work for bg/autovacuum workers