pg_stat_tmp files for dropped databases

Started by Thom Brownalmost 12 years ago4 messages
#1Thom Brown
thom@linux.com

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?

--
Thom

#2Tomas Vondra
tv@fuzzy.cz
In reply to: Thom Brown (#1)
1 attachment(s)
Re: 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

Attachments:

pgstat-dropdb-fix.patchtext/x-diff; name=pgstat-dropdb-fix.patchDownload
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);
#3Thom Brown
thom@linux.com
In reply to: Tomas Vondra (#2)
Re: pg_stat_tmp files for dropped databases

On 22 February 2014 01:07, Tomas Vondra <tv@fuzzy.cz> 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

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tomas Vondra (#2)
Re: 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 Herrera http://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