Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Tom Lane wrote:
>>> It doesn't seem to me that it'd be hard to support two locations for the
>>> stats file --- it'd just take another parameter to the read and write
>>> routines. pgstat.c already knows the difference between a normal write
>>> and a shutdown write ...
>
>> Right. Should it be removed from the permanent location when the server
>> starts?
>
> Yes, I would say so. There are two possible exit paths: normal shutdown
> (where we'd write a new file) and crash. In a crash we'd wish to delete
> the file anyway for fear that it's corrupted.
>
> Startup: read permanent file, then delete it.
>
> Post-crash: remove any permanent file (same as now)
>
> Shutdown: write permanent file.
>
> Normal stats collector write: write temp file.
>
> Backend stats fetch: read temp file.
Attached is a patch that implements this. I went with the option of just
storing it in a temporary directory that can be symlinked, and not
bothering with a GUC for it. Comments? (documentation updates are also
needed, but I'll wait with those until I hear patch comments :-P)
//Magnus
Index: backend/postmaster/pgstat.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.176
diff -c -r1.176 pgstat.c
*** backend/postmaster/pgstat.c 30 Jun 2008 10:58:47 -0000 1.176
--- backend/postmaster/pgstat.c 4 Aug 2008 09:39:23 -0000
***************
*** 67,74 ****
* Paths for the statistics files (relative to installation's $PGDATA).
* ----------
*/
! #define PGSTAT_STAT_FILENAME "global/pgstat.stat"
! #define PGSTAT_STAT_TMPFILE "global/pgstat.tmp"
/* ----------
* Timer definitions.
--- 67,76 ----
* Paths for the statistics files (relative to installation's $PGDATA).
* ----------
*/
! #define PGSTAT_STAT_PERMANENT_FILENAME "global/pgstat.stat"
! #define PGSTAT_STAT_PERMANENT_TMPFILE "global/pgstat.tmp"
! #define PGSTAT_STAT_FILENAME "pgstat_tmp/pgstat.stat"
! #define PGSTAT_STAT_TMPFILE "pgstat_tmp/pgstat.tmp"
/* ----------
* Timer definitions.
***************
*** 218,225 ****
static void pgstat_beshutdown_hook(int code, Datum arg);
static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
! static void pgstat_write_statsfile(void);
! static HTAB *pgstat_read_statsfile(Oid onlydb);
static void backend_read_statsfile(void);
static void pgstat_read_current_status(void);
--- 220,227 ----
static void pgstat_beshutdown_hook(int code, Datum arg);
static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
! static void pgstat_write_statsfile(bool permanent);
! static HTAB *pgstat_read_statsfile(Oid onlydb, bool permanent);
static void backend_read_statsfile(void);
static void pgstat_read_current_status(void);
***************
*** 509,514 ****
--- 511,517 ----
pgstat_reset_all(void)
{
unlink(PGSTAT_STAT_FILENAME);
+ unlink(PGSTAT_STAT_PERMANENT_FILENAME);
}
#ifdef EXEC_BACKEND
***************
*** 2595,2601 ****
* zero.
*/
pgStatRunningInCollector = true;
! pgStatDBHash = pgstat_read_statsfile(InvalidOid);
/*
* Setup the descriptor set for select(2). Since only one bit in the set
--- 2598,2604 ----
* zero.
*/
pgStatRunningInCollector = true;
! pgStatDBHash = pgstat_read_statsfile(InvalidOid, true);
/*
* Setup the descriptor set for select(2). Since only one bit in the set
***************
*** 2635,2641 ****
if (!PostmasterIsAlive(true))
break;
! pgstat_write_statsfile();
need_statwrite = false;
need_timer = true;
}
--- 2638,2644 ----
if (!PostmasterIsAlive(true))
break;
! pgstat_write_statsfile(false);
need_statwrite = false;
need_timer = true;
}
***************
*** 2803,2809 ****
/*
* Save the final stats to reuse at next startup.
*/
! pgstat_write_statsfile();
exit(0);
}
--- 2806,2812 ----
/*
* Save the final stats to reuse at next startup.
*/
! pgstat_write_statsfile(true);
exit(0);
}
***************
*** 2891,2897 ****
* ----------
*/
static void
! pgstat_write_statsfile(void)
{
HASH_SEQ_STATUS hstat;
HASH_SEQ_STATUS tstat;
--- 2894,2900 ----
* ----------
*/
static void
! pgstat_write_statsfile(bool permanent)
{
HASH_SEQ_STATUS hstat;
HASH_SEQ_STATUS tstat;
***************
*** 2901,2917 ****
PgStat_StatFuncEntry *funcentry;
FILE *fpout;
int32 format_id;
/*
* Open the statistics temp file to write out the current values.
*/
! fpout = fopen(PGSTAT_STAT_TMPFILE, PG_BINARY_W);
if (fpout == NULL)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not open temporary statistics file \"%s\": %m",
! PGSTAT_STAT_TMPFILE)));
return;
}
--- 2904,2922 ----
PgStat_StatFuncEntry *funcentry;
FILE *fpout;
int32 format_id;
+ const char *tmpfile = permanent?PGSTAT_STAT_PERMANENT_TMPFILE:PGSTAT_STAT_TMPFILE;
+ const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:PGSTAT_STAT_FILENAME;
/*
* Open the statistics temp file to write out the current values.
*/
! fpout = fopen(tmpfile, PG_BINARY_W);
if (fpout == NULL)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not open temporary statistics file \"%s\": %m",
! tmpfile)));
return;
}
***************
*** 2978,3002 ****
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not write temporary statistics file \"%s\": %m",
! PGSTAT_STAT_TMPFILE)));
fclose(fpout);
! unlink(PGSTAT_STAT_TMPFILE);
}
else if (fclose(fpout) < 0)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not close temporary statistics file \"%s\": %m",
! PGSTAT_STAT_TMPFILE)));
! unlink(PGSTAT_STAT_TMPFILE);
}
! else if (rename(PGSTAT_STAT_TMPFILE, PGSTAT_STAT_FILENAME) < 0)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not rename temporary statistics file \"%s\" to \"%s\": %m",
! PGSTAT_STAT_TMPFILE, PGSTAT_STAT_FILENAME)));
! unlink(PGSTAT_STAT_TMPFILE);
}
}
--- 2983,3007 ----
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not write temporary statistics file \"%s\": %m",
! tmpfile)));
fclose(fpout);
! unlink(tmpfile);
}
else if (fclose(fpout) < 0)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not close temporary statistics file \"%s\": %m",
! tmpfile)));
! unlink(tmpfile);
}
! else if (rename(tmpfile, statfile) < 0)
{
ereport(LOG,
(errcode_for_file_access(),
errmsg("could not rename temporary statistics file \"%s\" to \"%s\": %m",
! tmpfile, statfile)));
! unlink(tmpfile);
}
}
***************
*** 3006,3015 ****
*
* Reads in an existing statistics collector file and initializes the
* databases' hash table (whose entries point to the tables' hash tables).
* ----------
*/
static HTAB *
! pgstat_read_statsfile(Oid onlydb)
{
PgStat_StatDBEntry *dbentry;
PgStat_StatDBEntry dbbuf;
--- 3011,3025 ----
*
* Reads in an existing statistics collector file and initializes the
* databases' hash table (whose entries point to the tables' hash tables).
+ *
+ * If reading from the permanent file (which happens during collector
+ * startup, but never from backends), the file is removed once it's been
+ * successfully read. The temporary file is also removed at this time,
+ * to make sure backends don't read data from previous runs.
* ----------
*/
static HTAB *
! pgstat_read_statsfile(Oid onlydb, bool permanent)
{
PgStat_StatDBEntry *dbentry;
PgStat_StatDBEntry dbbuf;
***************
*** 3024,3029 ****
--- 3034,3040 ----
FILE *fpin;
int32 format_id;
bool found;
+ const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:PGSTAT_STAT_FILENAME;
/*
* The tables will live in pgStatLocalContext.
***************
*** 3052,3058 ****
* return zero for anything and the collector simply starts from scratch
* with empty counters.
*/
! if ((fpin = AllocateFile(PGSTAT_STAT_FILENAME, PG_BINARY_R)) == NULL)
return dbhash;
/*
--- 3063,3069 ----
* return zero for anything and the collector simply starts from scratch
* with empty counters.
*/
! if ((fpin = AllocateFile(statfile, PG_BINARY_R)) == NULL)
return dbhash;
/*
***************
*** 3241,3246 ****
--- 3252,3263 ----
done:
FreeFile(fpin);
+ if (permanent)
+ {
+ unlink(PGSTAT_STAT_PERMANENT_FILENAME);
+ unlink(PGSTAT_STAT_FILENAME);
+ }
+
return dbhash;
}
***************
*** 3259,3267 ****
/* Autovacuum launcher wants stats about all databases */
if (IsAutoVacuumLauncherProcess())
! pgStatDBHash = pgstat_read_statsfile(InvalidOid);
else
! pgStatDBHash = pgstat_read_statsfile(MyDatabaseId);
}
--- 3276,3284 ----
/* Autovacuum launcher wants stats about all databases */
if (IsAutoVacuumLauncherProcess())
! pgStatDBHash = pgstat_read_statsfile(InvalidOid, false);
else
! pgStatDBHash = pgstat_read_statsfile(MyDatabaseId, false);
}
Index: bin/initdb/initdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.158
diff -c -r1.158 initdb.c
*** bin/initdb/initdb.c 19 Jul 2008 04:01:29 -0000 1.158
--- bin/initdb/initdb.c 4 Aug 2008 09:39:23 -0000
***************
*** 2461,2467 ****
"pg_multixact/offsets",
"base",
"base/1",
! "pg_tblspc"
};
progname = get_progname(argv[0]);
--- 2461,2468 ----
"pg_multixact/offsets",
"base",
"base/1",
! "pg_tblspc",
! "pgstat_tmp"
};
progname = get_progname(argv[0]);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
