Thread: WIP: getting rid of the pg_database flat file

In the discussion of bug #4919 I wrote:
> In some sense this is a bootstrap problem: what does it take to get to
> the point of being able to read pg_database and its indexes?  That is
> necessarily not dependent on the particular database we want to join.
> Maybe we could solve it by having the relcache write a "global" cache
> file containing only entries for the global tables, and load that before
> we have identified the database we want to join (after which, we'll load
> another cache file for the local entries).  It would doubtless take some
> rearrangement of the backend startup sequence, but it doesn't seem
> obviously impossible.

Attached is a proof-of-concept patch which shows that this idea makes it
possible to start backends without the pg_database flat file, and that the
required search of pg_database can be done with an index as long as we
have the shared relcache cache file available (which should always be true
except for the first backend start after postmaster bootup or crash
recovery).  There are a few loose ends yet to fix, but on the whole it
was easier than I expected.  The main costs of doing it this way are:

* pg_database has to become a nailed-in-cache relation, as does its
index on datname.  (Its index on OID will have to be nailed too, unless
we can get rid of the kluge that lets autovacuum give InitPostgres a
database OID instead of database name.  I have not looked at autovacuum
yet.)  This doesn't really cost anything except a few more bytes in the
relcache ... and in reality I suspect pg_database is always in that
cache anyway.

* We have to have a Schema_pg_database macro in pg_attribute.h.  This
means a little more hand maintenance (unless we accept Robert Haas'
patch to autogenerate all that stuff); but it's still not a big problem.

I think this is clearly worth cleaning up and committing, since even
without any further progress it eliminates number-of-databases as a
significant factor in backend startup time.  Does anyone have any
objection to the above side-effects?

To actually get rid of the pg_database flat file, we'd need to take the
further step of teaching the AV launcher to read pg_database for itself,
or else refactor things so that the AV workers can do that for it.
(Alvaro, any comments about the best way to proceed there?)

I'd also like to look into getting rid of the pg_auth flat file.
As previously noted, that means postponing client auth to later in the
startup sequence.  If we were willing to eliminate role membership as an
available piece of information for auth method selection, we could still
do much of the auth work before initializing the backend proper; in
particular we could issue a password challenge and wait for a response,
which would be good in terms of reducing our exposure to lightweight DDOS
attacks.  I'm not sure if anyone will think that's a good tradeoff though,
since any attacker who can connect to the postmaster port can probably
DDOS the postmaster just fine anyway.

Comments?

            regards, tom lane

Tom Lane wrote:
> I'd also like to look into getting rid of the pg_auth flat file.
>   

That would be sad for many users of pgbouncer.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> I'd also like to look into getting rid of the pg_auth flat file.

> That would be sad for many users of pgbouncer.

Really?  And how would pgbouncer be depending on that?
        regards, tom lane

On Wednesday 12 August 2009 02:30:53 Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Tom Lane wrote:
> >> I'd also like to look into getting rid of the pg_auth flat file.
> > That would be sad for many users of pgbouncer.
> Really?  And how would pgbouncer be depending on that?
One can currently use a copy of that file for its authentication (which it does 
seperately).
I actually don't see a big problem there  - if its contents change often its 
not safe to use that file from outside anyway, and in the other case you easily 
can update it with a select...

Andres

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>   
>> Tom Lane wrote:
>>     
>>> I'd also like to look into getting rid of the pg_auth flat file.
>>>       
>
>   
>> That would be sad for many users of pgbouncer.
>>     
>
> Really?  And how would pgbouncer be depending on that?
>   

"

==== auth_file ====

The name of the file to load user names and passwords from. The file format 
is the same as the PostgreSQL pg_auth/pg_pwd file, so this setting can be 
pointed directly to one of those backend files.

"

see
<http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pgbouncer/pgbouncer/doc/config.txt?rev=1.21&content-type=text/x-cvsweb-markup>

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> Really?  And how would pgbouncer be depending on that?

> ==== auth_file ====

> The name of the file to load user names and passwords from. The file format 
> is the same as the PostgreSQL pg_auth/pg_pwd file, so this setting can be 
> pointed directly to one of those backend files.

Considering that we feel free to whack that file format around, and that
it's stored in a directory that non-postgres users shouldn't even be
able to read, I think this argument is just silly.  The only alternative
fix that was proposed in this thread
http://archives.postgresql.org/pgsql-performance/2009-08/msg00094.php
involved changing the flat files into a format that would definitely not
be easily readable by other programs, anyway.
        regards, tom lane

Tom Lane wrote:

> To actually get rid of the pg_database flat file, we'd need to take the
> further step of teaching the AV launcher to read pg_database for itself,
> or else refactor things so that the AV workers can do that for it.
> (Alvaro, any comments about the best way to proceed there?)

Hmm.  I don't see any easy way out of that at the moment ... the
launcher would have to become a pseudo-backend, at least to the point
where it is able to read pg_database.  I don't see how could workers
help the launcher with that, unless we made them write a flatfile
representation of pg_database, which would put us back where we started ...

I'll have a deeper look around.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> To actually get rid of the pg_database flat file, we'd need to take the
>> further step of teaching the AV launcher to read pg_database for itself,
>> or else refactor things so that the AV workers can do that for it.
>> (Alvaro, any comments about the best way to proceed there?)

> Hmm.  I don't see any easy way out of that at the moment ... the
> launcher would have to become a pseudo-backend, at least to the point
> where it is able to read pg_database.  I don't see how could workers
> help the launcher with that, unless we made them write a flatfile
> representation of pg_database, which would put us back where we started ...

> I'll have a deeper look around.

What was sort of in the back of my mind was to have every n'th AV worker
examine pg_database and report back to the launcher (probably through
shared memory) with an indication of the next few databases that should
be vacuumed and when.  Not sure how inefficient that might be though.
Is there a real downside to promoting the launcher to be a
pseudo-backend?
        regards, tom lane

Tom Lane wrote:

> What was sort of in the back of my mind was to have every n'th AV worker
> examine pg_database and report back to the launcher (probably through
> shared memory) with an indication of the next few databases that should
> be vacuumed and when.  Not sure how inefficient that might be though.

Hmm, probably we could do this.  The one open question is how would it
know what to do the first time around when there's no information set up
yet, so it cannot start a worker.  I guess we could hardcode to start a
worker in database with Id 1, but that doesn't seem like an improvement.

> Is there a real downside to promoting the launcher to be a
> pseudo-backend?

Aside from the fact that we don't have any pseudo-backend yet, I don't
see any ...

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> Is there a real downside to promoting the launcher to be a
>> pseudo-backend?

> Aside from the fact that we don't have any pseudo-backend yet, I don't
> see any ...

Well, I meant pseudo-backend in the sense of "just like an AV worker".
We might not want it to show in pg_stat_activity, but otherwise I think
it'd be the same.
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Tom Lane wrote:
> >> Is there a real downside to promoting the launcher to be a
> >> pseudo-backend?
> 
> > Aside from the fact that we don't have any pseudo-backend yet, I don't
> > see any ...
> 
> Well, I meant pseudo-backend in the sense of "just like an AV worker".
> We might not want it to show in pg_stat_activity, but otherwise I think
> it'd be the same.

Hmm, to what database would it connect?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> Well, I meant pseudo-backend in the sense of "just like an AV worker".
>> We might not want it to show in pg_stat_activity, but otherwise I think
>> it'd be the same.

> Hmm, to what database would it connect?

Well, it wouldn't.  As of the patch I'm working on, it's okay to have
PGPROC entries showing zero in databaseId.  Normally they'd be backends
that weren't done starting yet, but I see no reason the AV launcher
couldn't stay that way indefinitely.

This would likely mean that the only thing it could safely do is seqscan
pg_database, but that's all we need anyway, no?

I suppose it might be a bit ugly to refactor InitPostgres enough to
support this.  I'm not sure which parts of that code we'd actually need
to have run.
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:

> > Hmm, to what database would it connect?
> 
> Well, it wouldn't.  As of the patch I'm working on, it's okay to have
> PGPROC entries showing zero in databaseId.  Normally they'd be backends
> that weren't done starting yet, but I see no reason the AV launcher
> couldn't stay that way indefinitely.

Makes sense.

> This would likely mean that the only thing it could safely do is seqscan
> pg_database, but that's all we need anyway, no?

Right.

> I suppose it might be a bit ugly to refactor InitPostgres enough to
> support this.  I'm not sure which parts of that code we'd actually need
> to have run.

I'll have a look at that.

Are you going to commit the current patch?  We can remove the hacks that
support autovacuum later.  I was thinking that InitPostgres could be
split in two, with the first half ending just after
RelationCacheInitializePhase2.  Then workers could figure out their
database names and go off on the second half; regular backends would
just call the two halves directly.  That way, the launcher could use the
first half.  (BTW I assume there is going to be an index on OID
available on pg_database after the shared relcache initialization?)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Are you going to commit the current patch?  We can remove the hacks that
> support autovacuum later.  I was thinking that InitPostgres could be
> split in two, with the first half ending just after
> RelationCacheInitializePhase2.  Then workers could figure out their
> database names and go off on the second half; regular backends would
> just call the two halves directly.  That way, the launcher could use the
> first half.

Yeah, I intend to commit what I have after I finish cleaning up a few
loose ends (just found a bug with CLOBBER_CACHE_ALWAYS), and then we
can take a look at making autovacuum play with it.  I envision three
or four commits before flatfiles.c can disappear.

> (BTW I assume there is going to be an index on OID
> available on pg_database after the shared relcache initialization?)

Yeah.  That's not in the patch I sent last night, but the OID index has
to become nailed too in order to allow removing FindMyDatabaseByOid.
However, I'm not sure that the AV launcher could touch it --- were
you thinking that would be important?
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:

> > (BTW I assume there is going to be an index on OID
> > available on pg_database after the shared relcache initialization?)
> 
> Yeah.  That's not in the patch I sent last night, but the OID index has
> to become nailed too in order to allow removing FindMyDatabaseByOid.
> However, I'm not sure that the AV launcher could touch it --- were
> you thinking that would be important?

No, that's for the workers.  The launcher needs all the entries anyway.
(I'm assuming it will be able to check visibility of tuples, correct?
Hmm, it will need to run transactions in order to do that ...)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> No, that's for the workers.  The launcher needs all the entries anyway.
> (I'm assuming it will be able to check visibility of tuples, correct?
> Hmm, it will need to run transactions in order to do that ...)

No, you don't need to be in a transaction to check visibility.
flatfiles.c is already doing that without being in a transaction,
in some cases.  What you need a transaction for is to be able to
hold AccessShareLock on the table you're scanning.  For pg_database
it's not possible for any schema changes (like relfilenode changes)
to happen anyway.  But it is possible for vacuum to try to truncate
away empty end pages, which could result in a failure of a concurrent
heapscan without a lock.  The code in flatfiles.c is safe because it's
only doing that during the startup process when nobody else could be
running vacuum, but I don't suppose we can assume that for the AV
launcher.

I just noticed that we are depending on flatfiles.c to do a couple of
things other than generate the flat files.  In particular, we piggyback
on its scan of pg_database to produce the initial transaction ID wrap
limit value, and we rely on it to find all the databases that
RelationCacheInitFileRemove needs to be called for.  I had been thinking
of replacing the latter bit with a directory search, but I don't see how
to get rid of scanning pg_database for the wrap limit setting.  So it
seems like there is still going to need to be a linear scan of
pg_database during system startup.  Annoying ...
        regards, tom lane

* Alvaro Herrera (alvherre@commandprompt.com) wrote:
> No, that's for the workers.  The launcher needs all the entries anyway.
> (I'm assuming it will be able to check visibility of tuples, correct?
> Hmm, it will need to run transactions in order to do that ...)

I realize I'm jumping into the middle of this with something unrelated.
My apologies.  A feature I've wanted a couple of times and which I know
other people have asked for is being able to schedule tasks to run at
certain times.  Similar to cron or scheduler, but in the database
instead of having to set up something outside.

I would expect it to use something similar to the autovacuum
launcher/worker system.  I'm wondering if it would be possible to
consider how other autovacuum-like systems might be added in the future
when doing these changes.
Thanks,
    Stephen