Re: [HACKERS] pg_stat_tmp files for dropped databases
Tomas Vondra wrote: > On 22.2.2014 01:13, Thom Brown wrote: > > I've noticed that files for dropped databases aren't removed from > > pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting > > Postgres, all the old database pg_stat_tmp files remain. > Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to > the temporary file (in pg_stat_tmp), it builds a path to the permanent > file in pg_stat. Pushed, thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_tmp files for dropped databases
On 22 February 2014 01:07, Tomas Vondra wrote: > Hi, > > On 22.2.2014 01:13, Thom Brown wrote: > > Hi, > > > > I've noticed that files for dropped databases aren't removed from > > pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting > > Postgres, all the old database pg_stat_tmp files remain. > > > > Shouldn't these be cleaned up? > > Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to > the temporary file (in pg_stat_tmp), it builds a path to the permanent > file in pg_stat. > > The permanent files don't exist at that moment, as the postmaster is > still runnig and only writes them at shutdown. So the unlink deletes > nothing (i.e. returns ENOENT), but the return value is ignored for all > the unlink() in pgstat.c. > > Patch for master attached, needs to be backpatched to 9.3. > > I'd bet the files survived restart because pg_stat_tmp was kept between > the restart. Postmaster walks through all databases, and moves their > stats to 'pg_stat' (i.e. removes them from pg_stat_tmp). On the start it > does the opposite (move pg_stat -> pg_stat_tmp). > > But it does not clear the pg_stat_tmp directory, i.e. the files will > stay there until removed manually. IIRC it was implemented like this on > purpose (what if someone pointed pg_stat_tmp to directory with other > files) - only the ownership etc. is checked. So this is expected. > > Assuming you have just stats in the directory, it's safe to remove the > contents of pg_stat_tmp before starting up the database. On systems > having pg_stat_tmp pointed to tmpfs, this is basically what happens when > rebooting the machine (and why I haven't noticed this on our production > systems so far). > Thanks for confirming and for finding the explanation. -- Thom
Re: [HACKERS] pg_stat_tmp files for dropped databases
Hi,
On 22.2.2014 01:13, Thom Brown wrote:
> Hi,
>
> I've noticed that files for dropped databases aren't removed from
> pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting
> Postgres, all the old database pg_stat_tmp files remain.
>
> Shouldn't these be cleaned up?
Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to
the temporary file (in pg_stat_tmp), it builds a path to the permanent
file in pg_stat.
The permanent files don't exist at that moment, as the postmaster is
still runnig and only writes them at shutdown. So the unlink deletes
nothing (i.e. returns ENOENT), but the return value is ignored for all
the unlink() in pgstat.c.
Patch for master attached, needs to be backpatched to 9.3.
I'd bet the files survived restart because pg_stat_tmp was kept between
the restart. Postmaster walks through all databases, and moves their
stats to 'pg_stat' (i.e. removes them from pg_stat_tmp). On the start it
does the opposite (move pg_stat -> pg_stat_tmp).
But it does not clear the pg_stat_tmp directory, i.e. the files will
stay there until removed manually. IIRC it was implemented like this on
purpose (what if someone pointed pg_stat_tmp to directory with other
files) - only the ownership etc. is checked. So this is expected.
Assuming you have just stats in the directory, it's safe to remove the
contents of pg_stat_tmp before starting up the database. On systems
having pg_stat_tmp pointed to tmpfs, this is basically what happens when
rebooting the machine (and why I haven't noticed this on our production
systems so far).
regards
Tomas
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 305d126..7c075ef 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -4744,7 +4744,7 @@ pgstat_recv_dropdb(PgStat_MsgDropdb *msg, int len)
{
char statfile[MAXPGPATH];
- get_dbstat_filename(true, false, dbid, statfile, MAXPGPATH);
+ get_dbstat_filename(false, false, dbid, statfile, MAXPGPATH);
elog(DEBUG2, "removing %s", statfile);
unlink(statfile);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
