Re: cannot read pg_class without having selected a database / is this a bug?

Hi,

I've written a simple extension that limits number of connection by
IP/db/user, and I do receive this exception:

psql: FATAL: cannot read pg_class without having selected a database

I've found this happens because the extension defines a client auth hook
that reads pg_stat_activity. The really interesting thing is that this
happens only when I start several backends 'at the same time' right
after the cluster is started. From that time, everything works just fine.

So it seems like a race condition or something like that.

I've prepared a simple testcase to demonstrate this issue - see the
files attached. I've put there several 'sleep' to demonstrate the timing
error.

All you need to do is this:

1) compile the extension (make install)
2) add the extension to shared_preload_libraries
3) restart the cluster
4) start two backends at the same time (within a second or so)

Tomas

2011/12/3 Tomas Vondra <tv(at)fuzzy(dot)cz>:
> psql: FATAL:  cannot read pg_class without having selected a database
>
> I've found this happens because the extension defines a client auth hook
> that reads pg_stat_activity. The really interesting thing is that this
> happens only when I start several backends 'at the same time' right
> after the cluster is started. From that time, everything works just fine.

I'm surprised this ever works. To read pg_stat_activity, you need a
relcache entry for it. And how will you build one without selecting a
database?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 3.12.2011 23:37, Robert Haas wrote:
> 2011/12/3 Tomas Vondra <tv(at)fuzzy(dot)cz>:
>> psql: FATAL: cannot read pg_class without having selected a database
>>
>> I've found this happens because the extension defines a client auth hook
>> that reads pg_stat_activity. The really interesting thing is that this
>> happens only when I start several backends 'at the same time' right
>> after the cluster is started. From that time, everything works just fine.
>
> I'm surprised this ever works. To read pg_stat_activity, you need a
> relcache entry for it. And how will you build one without selecting a
> database?

What do you mean by selecting a database?

I do select a database when executing a psql, but I guess you mean
something that initializes the relcache entry and that's probably
executed after the auth hook.

That might explain why it fails at first and then works just fine,
although it's a bit strange. Wouldn't that mean you can't access any
catalogs from the auth hook?

Tomas

Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
> That might explain why it fails at first and then works just fine,
> although it's a bit strange. Wouldn't that mean you can't access any
> catalogs from the auth hook?

It should be possible to access shared catalogs from an auth hook.
pg_stat_activity is neither shared nor a catalog. Like Robert,
I find it astonishing that this works ever, because the info needed
simply isn't available until you've connected to a particular database.
The fact that the view is actually defined the same in every database
doesn't enter into that ...

regards, tom lane

On 4.12.2011 05:19, Tom Lane wrote:
> Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
>> That might explain why it fails at first and then works just fine,
>> although it's a bit strange. Wouldn't that mean you can't access any
>> catalogs from the auth hook?
>
> It should be possible to access shared catalogs from an auth hook.
> pg_stat_activity is neither shared nor a catalog. Like Robert,
> I find it astonishing that this works ever, because the info needed
> simply isn't available until you've connected to a particular database.
> The fact that the view is actually defined the same in every database
> doesn't enter into that ...

Hmmm, I do admit this is the first time I play with these things
(relcache, catalogs ...) so closely. so there are obviously things I'm
not aware of. For example I'm a bit confused what is / is not a shared
catalogue. Thanks in advance for your patience.

Anyway, the code I posted does not fail because of pg_stat_activity, it
fails because it attempts for find the dbname/username for the backends
(read from pg_stat_activity).

I've removed pg_stat_activity (see the code below) and it still fails.
The reason is that get_database_name attempts to read pg_database, but
once it gets to ScanPgRelation in relcache.c it notices MyDatabaseID=0
and so the check fails

This is the simplified code:

if (status == STATUS_OK)
{
char * db;

LWLockAcquire(lock, LW_EXCLUSIVE);

sleep(1);

if (MyBackendId > 2) {
db = get_database_name(17000);
}

sleep(4);

LWLockRelease(lock);

}

If you start two backends at the same time, the first one gets ID=2 and
skips the get_database_name call, the second one gets ID=3 and calls the
function (and it fails). Subsequent calls work, because the first
backend initializes the relcache or something.

Tomas

Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
> On 4.12.2011 05:19, Tom Lane wrote:
>> It should be possible to access shared catalogs from an auth hook.
>> pg_stat_activity is neither shared nor a catalog. Like Robert,
>> I find it astonishing that this works ever, because the info needed
>> simply isn't available until you've connected to a particular database.
>> The fact that the view is actually defined the same in every database
>> doesn't enter into that ...

> Hmmm, I do admit this is the first time I play with these things
> (relcache, catalogs ...) so closely. so there are obviously things I'm
> not aware of. For example I'm a bit confused what is / is not a shared
> catalogue. Thanks in advance for your patience.

See pg_class.relisshared.

> Anyway, the code I posted does not fail because of pg_stat_activity, it
> fails because it attempts for find the dbname/username for the backends
> (read from pg_stat_activity).

Well, get_database_name tries to do a syscache lookup, and the syscache
infrastructure isn't working yet. It is possible to read a shared
catalog at this stage, but you have to use lower-level access mechanisms
--- for an example with some comments, look at GetDatabaseTuple in
postinit.c.

regards, tom lane

On 4.12.2011 17:10, Tom Lane wrote:
> Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
>> Anyway, the code I posted does not fail because of pg_stat_activity, it
>> fails because it attempts for find the dbname/username for the backends
>> (read from pg_stat_activity).
>
> Well, get_database_name tries to do a syscache lookup, and the syscache
> infrastructure isn't working yet. It is possible to read a shared
> catalog at this stage, but you have to use lower-level access mechanisms
> --- for an example with some comments, look at GetDatabaseTuple in
> postinit.c.

Great, this seems to work perfectly.

What about the pg_stat_activity - is it safe to access that from auth
hook or is that just a coincidence that it works (and might stop working
in the future)?

Tomas

Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
> What about the pg_stat_activity - is it safe to access that from auth
> hook or is that just a coincidence that it works (and might stop working
> in the future)?

It doesn't seem like a good thing to rely on. There's certainly no
testing being done that would cause us to notice if it stopped working
so early in backend startup.

regards, tom lane

On Sun, Dec 4, 2011 at 4:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
>> What about the pg_stat_activity - is it safe to access that from auth
>> hook or is that just a coincidence that it works (and might stop working
>> in the future)?
>
> It doesn't seem like a good thing to rely on.  There's certainly no
> testing being done that would cause us to notice if it stopped working
> so early in backend startup.

I'm still puzzled that Tomas got it working at all. If MyDatabaseId
hasn't been set yet, the how did we manage to build a relcache entry
for anything - let alone an unshared catalog?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I'm still puzzled that Tomas got it working at all. If MyDatabaseId
> hasn't been set yet, the how did we manage to build a relcache entry
> for anything - let alone an unshared catalog?

Well, he wasn't actually issuing a SQL query, just calling some of the
pgstat.c subroutines that underlie the view. It happens that the pgstat
module has no backend-local initialization (at the moment, and
discounting the issue of making the process's own pgstat_activity entry),
so they were happy enough. It was the syscache stuff that was spitting
up.

regards, tom lane

On Mon, Dec 5, 2011 at 10:46 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I'm still puzzled that Tomas got it working at all.  If MyDatabaseId
>> hasn't been set yet, the how did we manage to build a relcache entry
>> for anything - let alone an unshared catalog?
>
> Well, he wasn't actually issuing a SQL query, just calling some of the
> pgstat.c subroutines that underlie the view.  It happens that the pgstat
> module has no backend-local initialization (at the moment, and
> discounting the issue of making the process's own pgstat_activity entry),
> so they were happy enough.  It was the syscache stuff that was spitting
> up.

Oh, I see.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company