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

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>
Subject: Re: %d in log_line_prefix doesn't work for bg/autovacuum workers
Date: 2014-05-17 20:34:04
Message-ID: 20140517203404.GB4484@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-05-17 16:23:26 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-05-17 12:23:51 -0400, Tom Lane wrote:
> >> Given that there haven't been complaints in the past ten years about how
> >> you can't rename an active database, I'm OK personally with locking this
> >> down forever. But I wonder if anyone wants to argue the contrary.
>
> > I don't see much need to allow it. But even if we're interested in
> > allowing rename properly there'll definitely be the need to update the
> > database name used in log messages.
>
> I think the client-side issues are far worse.

No disagreements from me there. Those just aren't particulary related to
MyDatabaseName existing...

> For example, if we allow
> renaming active databases then the subprocesses in a parallel pg_dump or
> pg_restore could connect to the wrong database, ie not the one the leader
> process is connected to. The very best-case outcome of that is confusing
> error messages, and worst-case could be pretty nasty (perhaps even
> security issues?).

Yea, I am pretty sure there's some nastyness that way. Don't really want
to go there for a feature that's not even been requested.

> We could no doubt teach pg_dump and pg_restore to
> cross-check database OIDs after reconnecting, but lots of other
> applications could have comparable problems.

I guess pg_dump would have to lock the database before dumping....

> >> If we do this at all, I think actually that is a good idea.
> >> MyProcPort->database_name is a *requested* db name, but arguably the
> >> %d log line field should represent the *actual* connected db name,
> >> which means it shouldn't start being reported until we have some
> >> confidence that such a DB exists.
>
> > Hm. I tentatively think that it's still reasonable to report it
> > earlier. There's a bunch of things (option processing, authentication)
> > that can error out before the database name is finally validated. It
> > seems somewhat helpful to give context there.
>
> What context? By definition, no such processing can depend on which
> database you're trying to connect to.

With context I mean printing a value for log_line_prefix's '%d'. If you
have a application constantly connecting with the wrong username it
might be easier to diagnose if you know the database it's trying to
connect to.

> > The reason I placed it where I did is that it's the first location where
> > we can be sure the database conected to is the correct one because now
> > the lock on the database is held and we've rechecked the name.
>
> No, if we've completed the previous lookup then the DB exists. The
> recheck is to catch the possibility that a rename or drop is completing
> concurrently --- but I don't think it's unreasonable to show the
> database's (old) name as soon as we've seen evidence that it does/did
> exist.

Fair enough. It's imo a minor judgement call with reasons for going
either way.

Greetings,

Andres Freund

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2014-05-17 20:35:29 Re: buildfarm animals and 'snapshot too old'
Previous Message Tomas Vondra 2014-05-17 20:33:31 Re: buildfarm: strange OOM failures on markhor (running CLOBBER_CACHE_RECURSIVELY)