Tomas Vondra wrote: > diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c > index be3adf1..4ec485e 100644 > --- a/src/backend/postmaster/pgstat.c > +++ b/src/backend/postmaster/pgstat.c > @@ -64,10 +64,14 @@ > > /* ---------- > * Paths for the statistics files (relative to installation's $PGDATA). > + * Permanent and temprorary, global and per-database files.
Note typo in the line above. > -#define PGSTAT_STAT_PERMANENT_FILENAME "global/pgstat.stat" > -#define PGSTAT_STAT_PERMANENT_TMPFILE "global/pgstat.tmp" > +#define PGSTAT_STAT_PERMANENT_DIRECTORY "stat" > +#define PGSTAT_STAT_PERMANENT_FILENAME "stat/global.stat" > +#define PGSTAT_STAT_PERMANENT_TMPFILE "stat/global.tmp" > +#define PGSTAT_STAT_PERMANENT_DB_FILENAME "stat/%d.stat" > +#define PGSTAT_STAT_PERMANENT_DB_TMPFILE "stat/%d.tmp" > +char *pgstat_stat_directory = NULL; > char *pgstat_stat_filename = NULL; > char *pgstat_stat_tmpname = NULL; > +char *pgstat_stat_db_filename = NULL; > +char *pgstat_stat_db_tmpname = NULL; I don't like the quoted parts very much; it seems awkward to have the snprintf patterns in one place and have them be used in very distant places. Is there a way to improve that? Also, if I understand clearly, the pgstat_stat_db_filename value needs to be an snprintf pattern too, right? What if it doesn't contain the required % specifier? Also, if you can filter this through pgindent, that would be best. Make sure to add DBWriteRequest to src/tools/pgindent/typedefs_list. > + /* > + * There's no request for this DB yet, so lets create it > (allocate a > + * space for it, set the values). > + */ > + if (last_statrequests == NULL) > + last_statrequests = palloc(sizeof(DBWriteRequest)); > + else > + last_statrequests = repalloc(last_statrequests, > + > (num_statrequests + 1)*sizeof(DBWriteRequest)); > + > + last_statrequests[num_statrequests].databaseid = > msg->databaseid; > + last_statrequests[num_statrequests].request_time = > msg->clock_time; > + num_statrequests += 1; Having to repalloc this array each time seems wrong. Would a list instead of an array help? see ilist.c/h; I vote for a dlist because you can easily delete elements from the middle of it, if required (I think you'd need that.) > + char db_statfile[strlen(pgstat_stat_db_filename) + 11]; > + snprintf(db_statfile, strlen(pgstat_stat_db_filename) + 11, > + pgstat_stat_filename, dbentry->databaseid); This pattern seems rather frequent. Can we use a macro or similar here? Encapsulating the "11" better would be good. Magic numbers are evil. > diff --git a/src/include/pgstat.h b/src/include/pgstat.h > index 613c1c2..b3467d2 100644 > --- a/src/include/pgstat.h > +++ b/src/include/pgstat.h > @@ -205,6 +205,7 @@ typedef struct PgStat_MsgInquiry > PgStat_MsgHdr m_hdr; > TimestampTz clock_time; /* observed local clock time */ > TimestampTz cutoff_time; /* minimum acceptable file timestamp */ > + Oid databaseid; /* requested DB > (InvalidOid => all DBs) */ > } PgStat_MsgInquiry; Do we need to support the case that somebody requests stuff from the "shared" DB? IIRC that's what InvalidOid means in pgstat ... -- Á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
