PATCH: tracking temp files in pg_stat_database

Started by Tomas Vondraabout 14 years ago12 messages
#1Tomas Vondra
tv@fuzzy.cz
1 attachment(s)

Hello everybody,

this patch adds two counters to pg_stat_database to track temporary
files - number of temp files and number of bytes. I see this as a useful
feature, as temporary files often cause a lot of IO (because of low
work_mem etc.). The log_temp_files is useful, but you have to parse the
log and only temp files exceeding a size limit are logged so the actual
amount of I/O is unknown.

The patch is rather simple:

1) two new fields in PgStat_StatDBEntry (n_temp_files, n_temp_bytes)
2) report/recv function in pgstat.c
3) FileClose modified to log stats for all temp files (see below)
4) two new fields added to pg_stat_database (temp_files, temp_bytes)

I had to modify FileClose to call stat() on each temp file as this
should log all temp files (not just when log_temp_file >= 0). But the
impact of this change should be negligible, considering that the user is
already using temp files.

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Tomas

Attachments:

stats-temp-files.difftext/plain; name=stats-temp-files.diffDownload
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2253ca8..55d20dc 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -574,6 +574,8 @@ CREATE VIEW pg_stat_database AS
             pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
             pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
             pg_stat_get_db_conflict_all(D.oid) AS conflicts,
+            pg_stat_get_db_temp_files(D.oid) AS temp_files,
+            pg_stat_get_db_temp_bytes(D.oid) AS temp_bytes,
             pg_stat_get_db_stat_reset_time(D.oid) AS stats_reset
     FROM pg_database D;
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 24f4cde..97c7004 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -286,7 +286,7 @@ static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
 static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
 static void pgstat_recv_funcpurge(PgStat_MsgFuncpurge *msg, int len);
 static void pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len);
-
+static void pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len);
 
 /* ------------------------------------------------------------
  * Public functions called from postmaster follow
@@ -1339,6 +1339,29 @@ pgstat_report_recovery_conflict(int reason)
 	pgstat_send(&msg, sizeof(msg));
 }
 
+
+/* --------
+ * pgstat_report_tempfile() -
+ *
+ *	Tell the collector about a temporary file.
+ * --------
+ */
+void
+pgstat_report_tempfile(size_t filesize)
+{
+	PgStat_MsgTempFile msg;
+
+	if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+		return;
+
+	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_TEMPFILE);
+	msg.m_databaseid = MyDatabaseId;
+	msg.m_filesize = filesize;
+	pgstat_send(&msg, sizeof(msg));
+}
+
+;
+
 /* ----------
  * pgstat_ping() -
  *
@@ -3185,6 +3208,10 @@ PgstatCollectorMain(int argc, char *argv[])
 					pgstat_recv_recoveryconflict((PgStat_MsgRecoveryConflict *) &msg, len);
 					break;
 
+				case PGSTAT_MTYPE_TEMPFILE:
+					pgstat_recv_tempfile((PgStat_MsgTempFile *) &msg, len);
+					break;
+
 				default:
 					break;
 			}
@@ -3266,6 +3293,8 @@ pgstat_get_db_entry(Oid databaseid, bool create)
 		result->n_conflict_snapshot = 0;
 		result->n_conflict_bufferpin = 0;
 		result->n_conflict_startup_deadlock = 0;
+		result->n_temp_files = 0;
+		result->n_temp_bytes = 0;
 
 		result->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4177,6 +4206,8 @@ pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len)
 	dbentry->n_tuples_updated = 0;
 	dbentry->n_tuples_deleted = 0;
 	dbentry->last_autovac_time = 0;
+	dbentry->n_temp_bytes = 0;
+	dbentry->n_temp_files = 0;
 
 	dbentry->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4403,6 +4434,24 @@ pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len)
 }
 
 /* ----------
+ * pgstat_recv_tempfile() -
+ *
+ *	Process as PGSTAT_MTYPE_TEMPFILE message.
+ * ----------
+ */
+static void
+pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len)
+{
+	PgStat_StatDBEntry *dbentry;
+
+	dbentry = pgstat_get_db_entry(msg->m_databaseid, true);
+
+	dbentry->n_temp_bytes += msg->m_filesize;
+	dbentry->n_temp_files += 1;
+
+}
+
+/* ----------
  * pgstat_recv_funcstat() -
  *
  *	Count what the backend has done.
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 9540279..5e40d12 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1088,6 +1088,10 @@ FileClose(File file)
 	 */
 	if (vfdP->fdstate & FD_TEMPORARY)
 	{
+		
+		struct stat filestats;
+		int			stat_errno;
+		
 		/*
 		 * If we get an error, as could happen within the ereport/elog calls,
 		 * we'll come right back here during transaction abort.  Reset the
@@ -1101,23 +1105,23 @@ FileClose(File file)
 		temporary_files_size -= vfdP->fileSize;
 		vfdP->fileSize = 0;
 
-		if (log_temp_files >= 0)
-		{
-			struct stat filestats;
-			int			stat_errno;
+		/* first try the stat() */
+		if (stat(vfdP->fileName, &filestats))
+			stat_errno = errno;
+		else
+			stat_errno = 0;
 
-			/* first try the stat() */
-			if (stat(vfdP->fileName, &filestats))
-				stat_errno = errno;
-			else
-				stat_errno = 0;
+		/* in any case do the unlink */
+		if (unlink(vfdP->fileName))
+			elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
 
-			/* in any case do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
+		/* and last report the stat results */
+		if (stat_errno == 0)
+		{
 
-			/* and last report the stat results */
-			if (stat_errno == 0)
+			pgstat_report_tempfile(filestats.st_size);
+			
+			if (log_temp_files >= 0)
 			{
 				if ((filestats.st_size / 1024) >= log_temp_files)
 					ereport(LOG,
@@ -1131,12 +1135,6 @@ FileClose(File file)
 				elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
 			}
 		}
-		else
-		{
-			/* easy case, just do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
-		}
 	}
 
 	/* Unregister it from the resource owner */
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 7792b33..4eeac56 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -79,6 +79,8 @@ extern Datum pg_stat_get_db_conflict_bufferpin(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_startup_deadlock(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_all(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_files(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS);
 
 extern Datum pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_bgwriter_requested_checkpoints(PG_FUNCTION_ARGS);
@@ -1180,6 +1182,37 @@ pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS)
 }
 
 Datum
+pg_stat_get_db_temp_files(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_files;
+
+	PG_RETURN_INT64(result);
+}
+
+
+Datum
+pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_bytes;
+
+	PG_RETURN_INT64(result);
+}
+
+Datum
 pg_stat_get_db_conflict_tablespace(PG_FUNCTION_ARGS)
 {
 	Oid			dbid = PG_GETARG_OID(0);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 60e9feb..b4e5feb 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2626,6 +2626,10 @@ DATA(insert OID = 3070 (  pg_stat_get_db_conflict_all PGNSP PGUID 12 1 0 0 0 f f
 DESCR("statistics: recovery conflicts in database");
 DATA(insert OID = 3074 (  pg_stat_get_db_stat_reset_time PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 1184 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_stat_reset_time _null_ _null_ _null_ ));
 DESCR("statistics: last reset for a database");
+DATA(insert OID = 3079 (  pg_stat_get_db_temp_files PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_files _null_ _null_ _null_ ));
+DESCR("statistics: number of temporary files written");
+DATA(insert OID = 3080 (  pg_stat_get_db_temp_bytes PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_bytes _null_ _null_ _null_ ));
+DESCR("statistics: number of bytes in temporary files written");
 DATA(insert OID = 2769 ( pg_stat_get_bgwriter_timed_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_timed_checkpoints _null_ _null_ _null_ ));
 DESCR("statistics: number of timed checkpoints started by the bgwriter");
 DATA(insert OID = 2770 ( pg_stat_get_bgwriter_requested_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_requested_checkpoints _null_ _null_ _null_ ));
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 651b7d9..de775dd 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -47,7 +47,8 @@ typedef enum StatMsgType
 	PGSTAT_MTYPE_BGWRITER,
 	PGSTAT_MTYPE_FUNCSTAT,
 	PGSTAT_MTYPE_FUNCPURGE,
-	PGSTAT_MTYPE_RECOVERYCONFLICT
+	PGSTAT_MTYPE_RECOVERYCONFLICT,
+	PGSTAT_MTYPE_TEMPFILE
 } StatMsgType;
 
 /* ----------
@@ -377,6 +378,18 @@ typedef struct PgStat_MsgRecoveryConflict
 } PgStat_MsgRecoveryConflict;
 
 /* ----------
+ * PgStat_MsgTempFile	Sent by the backend upon creating a temp file
+ * ----------
+ */
+typedef struct PgStat_MsgTempFile
+{
+	PgStat_MsgHdr m_hdr;
+
+	Oid			m_databaseid;
+	size_t		m_filesize;
+} PgStat_MsgTempFile;
+
+/* ----------
  * PgStat_FunctionCounts	The actual per-function counts kept by a backend
  *
  * This struct should contain only actual event counters, because we memcmp
@@ -507,9 +520,11 @@ typedef struct PgStat_StatDBEntry
 	PgStat_Counter n_conflict_snapshot;
 	PgStat_Counter n_conflict_bufferpin;
 	PgStat_Counter n_conflict_startup_deadlock;
+	PgStat_Counter n_temp_files;
+	PgStat_Counter n_temp_bytes;
+	
 	TimestampTz stat_reset_timestamp;
 
-
 	/*
 	 * tables and functions must be last in the struct, because we don't write
 	 * the pointers out to the stats file.
@@ -716,6 +731,7 @@ extern void pgstat_initialize(void);
 extern void pgstat_bestart(void);
 
 extern void pgstat_report_activity(const char *cmd_str);
+extern void pgstat_report_tempfile(size_t filesize);
 extern void pgstat_report_appname(const char *appname);
 extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
 extern void pgstat_report_waiting(bool waiting);
#2Magnus Hagander
magnus@hagander.net
In reply to: Tomas Vondra (#1)
Re: PATCH: tracking temp files in pg_stat_database

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

Hello everybody,

this patch adds two counters to pg_stat_database to track temporary
files - number of temp files and number of bytes. I see this as a useful
feature, as temporary files often cause a lot of IO (because of low
work_mem etc.). The log_temp_files is useful, but you have to parse the
log and only temp files exceeding a size limit are logged so the actual
amount of I/O is unknown.

Hey, cool, that was on my personal TODO list :-)

The patch is rather simple:

1) two new fields in PgStat_StatDBEntry (n_temp_files, n_temp_bytes)
2) report/recv function in pgstat.c
3) FileClose modified to log stats for all temp files (see below)
4) two new fields added to pg_stat_database (temp_files, temp_bytes)

I haven't reviewed the code itself yet, but that seems like a
reasonable approach.

I had to modify FileClose to call stat() on each temp file as this
should log all temp files (not just when log_temp_file >= 0). But the
impact of this change should be negligible, considering that the user is
already using temp files.

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#3Tomas Vondra
tv@fuzzy.cz
In reply to: Magnus Hagander (#2)
Re: PATCH: tracking temp files in pg_stat_database

On 20 Prosinec 2011, 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

Hm, I added it to the current commit fest - I should probably move it to
the next one (2012-01), right? I'll add the docs at the evening.

Tomas

#4Magnus Hagander
magnus@hagander.net
In reply to: Tomas Vondra (#3)
Re: PATCH: tracking temp files in pg_stat_database

On Tue, Dec 20, 2011 at 11:45, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20 Prosinec 2011, 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

Hm, I added it to the current commit fest - I should probably move it to
the next one (2012-01), right? I'll add the docs at the evening.

Yes, all new patches should always go on the one that's labeled
"Open". If we don't do it that way, we will never finish a CF...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#5Tomas Vondra
tv@fuzzy.cz
In reply to: Magnus Hagander (#2)
1 attachment(s)
Re: PATCH: tracking temp files in pg_stat_database

On 20.12.2011 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

I've added the docs (see the attachment) and rebased to current head.

Tomas

Attachments:

stats-temp-files-v2.difftext/plain; name=stats-temp-files-v2.diffDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a12a9a2..3635c3f 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -691,6 +691,26 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
      </row>
 
      <row>
+      <entry><literal><function>pg_stat_get_db_temp_files</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Nuber of temporary files written for the database. All temporary files are
+       counted, regardless of why the temporary file was created (sorting or hash
+       join) or file size (log_temp_file does not affect this).
+      </entry>
+     </row>
+
+     <row>
+      <entry><literal><function>pg_stat_get_db_temp_bytes</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Amount of data written to temporary files for the database. All temporary
+       files are counted, regardless of why the temporary file was created (sorting
+       or hash join) or file size (log_temp_file does not affect this).
+      </entry>
+     </row>
+
+     <row>
       <entry><literal><function>pg_stat_get_numscans</function>(<type>oid</type>)</literal></entry>
       <entry><type>bigint</type></entry>
       <entry>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2253ca8..55d20dc 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -574,6 +574,8 @@ CREATE VIEW pg_stat_database AS
             pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
             pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
             pg_stat_get_db_conflict_all(D.oid) AS conflicts,
+            pg_stat_get_db_temp_files(D.oid) AS temp_files,
+            pg_stat_get_db_temp_bytes(D.oid) AS temp_bytes,
             pg_stat_get_db_stat_reset_time(D.oid) AS stats_reset
     FROM pg_database D;
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 24f4cde..97c7004 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -286,7 +286,7 @@ static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
 static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
 static void pgstat_recv_funcpurge(PgStat_MsgFuncpurge *msg, int len);
 static void pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len);
-
+static void pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len);
 
 /* ------------------------------------------------------------
  * Public functions called from postmaster follow
@@ -1339,6 +1339,29 @@ pgstat_report_recovery_conflict(int reason)
 	pgstat_send(&msg, sizeof(msg));
 }
 
+
+/* --------
+ * pgstat_report_tempfile() -
+ *
+ *	Tell the collector about a temporary file.
+ * --------
+ */
+void
+pgstat_report_tempfile(size_t filesize)
+{
+	PgStat_MsgTempFile msg;
+
+	if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+		return;
+
+	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_TEMPFILE);
+	msg.m_databaseid = MyDatabaseId;
+	msg.m_filesize = filesize;
+	pgstat_send(&msg, sizeof(msg));
+}
+
+;
+
 /* ----------
  * pgstat_ping() -
  *
@@ -3185,6 +3208,10 @@ PgstatCollectorMain(int argc, char *argv[])
 					pgstat_recv_recoveryconflict((PgStat_MsgRecoveryConflict *) &msg, len);
 					break;
 
+				case PGSTAT_MTYPE_TEMPFILE:
+					pgstat_recv_tempfile((PgStat_MsgTempFile *) &msg, len);
+					break;
+
 				default:
 					break;
 			}
@@ -3266,6 +3293,8 @@ pgstat_get_db_entry(Oid databaseid, bool create)
 		result->n_conflict_snapshot = 0;
 		result->n_conflict_bufferpin = 0;
 		result->n_conflict_startup_deadlock = 0;
+		result->n_temp_files = 0;
+		result->n_temp_bytes = 0;
 
 		result->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4177,6 +4206,8 @@ pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len)
 	dbentry->n_tuples_updated = 0;
 	dbentry->n_tuples_deleted = 0;
 	dbentry->last_autovac_time = 0;
+	dbentry->n_temp_bytes = 0;
+	dbentry->n_temp_files = 0;
 
 	dbentry->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4403,6 +4434,24 @@ pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len)
 }
 
 /* ----------
+ * pgstat_recv_tempfile() -
+ *
+ *	Process as PGSTAT_MTYPE_TEMPFILE message.
+ * ----------
+ */
+static void
+pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len)
+{
+	PgStat_StatDBEntry *dbentry;
+
+	dbentry = pgstat_get_db_entry(msg->m_databaseid, true);
+
+	dbentry->n_temp_bytes += msg->m_filesize;
+	dbentry->n_temp_files += 1;
+
+}
+
+/* ----------
  * pgstat_recv_funcstat() -
  *
  *	Count what the backend has done.
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 9540279..5e40d12 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1088,6 +1088,10 @@ FileClose(File file)
 	 */
 	if (vfdP->fdstate & FD_TEMPORARY)
 	{
+		
+		struct stat filestats;
+		int			stat_errno;
+		
 		/*
 		 * If we get an error, as could happen within the ereport/elog calls,
 		 * we'll come right back here during transaction abort.  Reset the
@@ -1101,23 +1105,23 @@ FileClose(File file)
 		temporary_files_size -= vfdP->fileSize;
 		vfdP->fileSize = 0;
 
-		if (log_temp_files >= 0)
-		{
-			struct stat filestats;
-			int			stat_errno;
+		/* first try the stat() */
+		if (stat(vfdP->fileName, &filestats))
+			stat_errno = errno;
+		else
+			stat_errno = 0;
 
-			/* first try the stat() */
-			if (stat(vfdP->fileName, &filestats))
-				stat_errno = errno;
-			else
-				stat_errno = 0;
+		/* in any case do the unlink */
+		if (unlink(vfdP->fileName))
+			elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
 
-			/* in any case do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
+		/* and last report the stat results */
+		if (stat_errno == 0)
+		{
 
-			/* and last report the stat results */
-			if (stat_errno == 0)
+			pgstat_report_tempfile(filestats.st_size);
+			
+			if (log_temp_files >= 0)
 			{
 				if ((filestats.st_size / 1024) >= log_temp_files)
 					ereport(LOG,
@@ -1131,12 +1135,6 @@ FileClose(File file)
 				elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
 			}
 		}
-		else
-		{
-			/* easy case, just do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
-		}
 	}
 
 	/* Unregister it from the resource owner */
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 7792b33..4eeac56 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -79,6 +79,8 @@ extern Datum pg_stat_get_db_conflict_bufferpin(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_startup_deadlock(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_all(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_files(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS);
 
 extern Datum pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_bgwriter_requested_checkpoints(PG_FUNCTION_ARGS);
@@ -1180,6 +1182,37 @@ pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS)
 }
 
 Datum
+pg_stat_get_db_temp_files(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_files;
+
+	PG_RETURN_INT64(result);
+}
+
+
+Datum
+pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_bytes;
+
+	PG_RETURN_INT64(result);
+}
+
+Datum
 pg_stat_get_db_conflict_tablespace(PG_FUNCTION_ARGS)
 {
 	Oid			dbid = PG_GETARG_OID(0);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index ffe0727..9ab9e40 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2626,6 +2626,10 @@ DATA(insert OID = 3070 (  pg_stat_get_db_conflict_all PGNSP PGUID 12 1 0 0 0 f f
 DESCR("statistics: recovery conflicts in database");
 DATA(insert OID = 3074 (  pg_stat_get_db_stat_reset_time PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 1184 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_stat_reset_time _null_ _null_ _null_ ));
 DESCR("statistics: last reset for a database");
+DATA(insert OID = 3079 (  pg_stat_get_db_temp_files PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_files _null_ _null_ _null_ ));
+DESCR("statistics: number of temporary files written");
+DATA(insert OID = 3080 (  pg_stat_get_db_temp_bytes PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_bytes _null_ _null_ _null_ ));
+DESCR("statistics: number of bytes in temporary files written");
 DATA(insert OID = 2769 ( pg_stat_get_bgwriter_timed_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_timed_checkpoints _null_ _null_ _null_ ));
 DESCR("statistics: number of timed checkpoints started by the bgwriter");
 DATA(insert OID = 2770 ( pg_stat_get_bgwriter_requested_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_requested_checkpoints _null_ _null_ _null_ ));
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 651b7d9..de775dd 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -47,7 +47,8 @@ typedef enum StatMsgType
 	PGSTAT_MTYPE_BGWRITER,
 	PGSTAT_MTYPE_FUNCSTAT,
 	PGSTAT_MTYPE_FUNCPURGE,
-	PGSTAT_MTYPE_RECOVERYCONFLICT
+	PGSTAT_MTYPE_RECOVERYCONFLICT,
+	PGSTAT_MTYPE_TEMPFILE
 } StatMsgType;
 
 /* ----------
@@ -377,6 +378,18 @@ typedef struct PgStat_MsgRecoveryConflict
 } PgStat_MsgRecoveryConflict;
 
 /* ----------
+ * PgStat_MsgTempFile	Sent by the backend upon creating a temp file
+ * ----------
+ */
+typedef struct PgStat_MsgTempFile
+{
+	PgStat_MsgHdr m_hdr;
+
+	Oid			m_databaseid;
+	size_t		m_filesize;
+} PgStat_MsgTempFile;
+
+/* ----------
  * PgStat_FunctionCounts	The actual per-function counts kept by a backend
  *
  * This struct should contain only actual event counters, because we memcmp
@@ -507,9 +520,11 @@ typedef struct PgStat_StatDBEntry
 	PgStat_Counter n_conflict_snapshot;
 	PgStat_Counter n_conflict_bufferpin;
 	PgStat_Counter n_conflict_startup_deadlock;
+	PgStat_Counter n_temp_files;
+	PgStat_Counter n_temp_bytes;
+	
 	TimestampTz stat_reset_timestamp;
 
-
 	/*
 	 * tables and functions must be last in the struct, because we don't write
 	 * the pointers out to the stats file.
@@ -716,6 +731,7 @@ extern void pgstat_initialize(void);
 extern void pgstat_bestart(void);
 
 extern void pgstat_report_activity(const char *cmd_str);
+extern void pgstat_report_tempfile(size_t filesize);
 extern void pgstat_report_appname(const char *appname);
 extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
 extern void pgstat_report_waiting(bool waiting);
#6Tomas Vondra
tv@fuzzy.cz
In reply to: Tomas Vondra (#5)
1 attachment(s)
Re: PATCH: tracking temp files in pg_stat_database

On 20.12.2011 19:59, Tomas Vondra wrote:

On 20.12.2011 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

I've added the docs (see the attachment) and rebased to current head.

Tomas

Fixed a failing regression test (check of pg_stat_database structure).

Tomas

Attachments:

stats-temp-files-v3.difftext/plain; name=stats-temp-files-v3.diffDownload
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a12a9a2..3635c3f 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -691,6 +691,26 @@ postgres: <replaceable>user</> <replaceable>database</> <replaceable>host</> <re
      </row>
 
      <row>
+      <entry><literal><function>pg_stat_get_db_temp_files</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Nuber of temporary files written for the database. All temporary files are
+       counted, regardless of why the temporary file was created (sorting or hash
+       join) or file size (log_temp_file does not affect this).
+      </entry>
+     </row>
+
+     <row>
+      <entry><literal><function>pg_stat_get_db_temp_bytes</function>(<type>oid</type>)</literal></entry>
+      <entry><type>bigint</type></entry>
+      <entry>
+       Amount of data written to temporary files for the database. All temporary
+       files are counted, regardless of why the temporary file was created (sorting
+       or hash join) or file size (log_temp_file does not affect this).
+      </entry>
+     </row>
+
+     <row>
       <entry><literal><function>pg_stat_get_numscans</function>(<type>oid</type>)</literal></entry>
       <entry><type>bigint</type></entry>
       <entry>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 50ba20c..3a0dca3 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -574,6 +574,8 @@ CREATE VIEW pg_stat_database AS
             pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
             pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
             pg_stat_get_db_conflict_all(D.oid) AS conflicts,
+            pg_stat_get_db_temp_files(D.oid) AS temp_files,
+            pg_stat_get_db_temp_bytes(D.oid) AS temp_bytes,
             pg_stat_get_db_stat_reset_time(D.oid) AS stats_reset
     FROM pg_database D;
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 323d42b..6a2ff1d 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -286,7 +286,7 @@ static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
 static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
 static void pgstat_recv_funcpurge(PgStat_MsgFuncpurge *msg, int len);
 static void pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len);
-
+static void pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len);
 
 /* ------------------------------------------------------------
  * Public functions called from postmaster follow
@@ -1339,6 +1339,29 @@ pgstat_report_recovery_conflict(int reason)
 	pgstat_send(&msg, sizeof(msg));
 }
 
+
+/* --------
+ * pgstat_report_tempfile() -
+ *
+ *	Tell the collector about a temporary file.
+ * --------
+ */
+void
+pgstat_report_tempfile(size_t filesize)
+{
+	PgStat_MsgTempFile msg;
+
+	if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+		return;
+
+	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_TEMPFILE);
+	msg.m_databaseid = MyDatabaseId;
+	msg.m_filesize = filesize;
+	pgstat_send(&msg, sizeof(msg));
+}
+
+;
+
 /* ----------
  * pgstat_ping() -
  *
@@ -3185,6 +3208,10 @@ PgstatCollectorMain(int argc, char *argv[])
 					pgstat_recv_recoveryconflict((PgStat_MsgRecoveryConflict *) &msg, len);
 					break;
 
+				case PGSTAT_MTYPE_TEMPFILE:
+					pgstat_recv_tempfile((PgStat_MsgTempFile *) &msg, len);
+					break;
+
 				default:
 					break;
 			}
@@ -3266,6 +3293,8 @@ pgstat_get_db_entry(Oid databaseid, bool create)
 		result->n_conflict_snapshot = 0;
 		result->n_conflict_bufferpin = 0;
 		result->n_conflict_startup_deadlock = 0;
+		result->n_temp_files = 0;
+		result->n_temp_bytes = 0;
 
 		result->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4177,6 +4206,8 @@ pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len)
 	dbentry->n_tuples_updated = 0;
 	dbentry->n_tuples_deleted = 0;
 	dbentry->last_autovac_time = 0;
+	dbentry->n_temp_bytes = 0;
+	dbentry->n_temp_files = 0;
 
 	dbentry->stat_reset_timestamp = GetCurrentTimestamp();
 
@@ -4403,6 +4434,24 @@ pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int len)
 }
 
 /* ----------
+ * pgstat_recv_tempfile() -
+ *
+ *	Process as PGSTAT_MTYPE_TEMPFILE message.
+ * ----------
+ */
+static void
+pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len)
+{
+	PgStat_StatDBEntry *dbentry;
+
+	dbentry = pgstat_get_db_entry(msg->m_databaseid, true);
+
+	dbentry->n_temp_bytes += msg->m_filesize;
+	dbentry->n_temp_files += 1;
+
+}
+
+/* ----------
  * pgstat_recv_funcstat() -
  *
  *	Count what the backend has done.
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 43bc43a..7a47ae7 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1088,6 +1088,10 @@ FileClose(File file)
 	 */
 	if (vfdP->fdstate & FD_TEMPORARY)
 	{
+		
+		struct stat filestats;
+		int			stat_errno;
+		
 		/*
 		 * If we get an error, as could happen within the ereport/elog calls,
 		 * we'll come right back here during transaction abort.  Reset the
@@ -1101,23 +1105,23 @@ FileClose(File file)
 		temporary_files_size -= vfdP->fileSize;
 		vfdP->fileSize = 0;
 
-		if (log_temp_files >= 0)
-		{
-			struct stat filestats;
-			int			stat_errno;
+		/* first try the stat() */
+		if (stat(vfdP->fileName, &filestats))
+			stat_errno = errno;
+		else
+			stat_errno = 0;
 
-			/* first try the stat() */
-			if (stat(vfdP->fileName, &filestats))
-				stat_errno = errno;
-			else
-				stat_errno = 0;
+		/* in any case do the unlink */
+		if (unlink(vfdP->fileName))
+			elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
 
-			/* in any case do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
+		/* and last report the stat results */
+		if (stat_errno == 0)
+		{
 
-			/* and last report the stat results */
-			if (stat_errno == 0)
+			pgstat_report_tempfile(filestats.st_size);
+			
+			if (log_temp_files >= 0)
 			{
 				if ((filestats.st_size / 1024) >= log_temp_files)
 					ereport(LOG,
@@ -1131,12 +1135,6 @@ FileClose(File file)
 				elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
 			}
 		}
-		else
-		{
-			/* easy case, just do the unlink */
-			if (unlink(vfdP->fileName))
-				elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
-		}
 	}
 
 	/* Unregister it from the resource owner */
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index b4986d8..2804af7 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -79,6 +79,8 @@ extern Datum pg_stat_get_db_conflict_bufferpin(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_startup_deadlock(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_conflict_all(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_files(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS);
 
 extern Datum pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS);
 extern Datum pg_stat_get_bgwriter_requested_checkpoints(PG_FUNCTION_ARGS);
@@ -1180,6 +1182,37 @@ pg_stat_get_db_stat_reset_time(PG_FUNCTION_ARGS)
 }
 
 Datum
+pg_stat_get_db_temp_files(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_files;
+
+	PG_RETURN_INT64(result);
+}
+
+
+Datum
+pg_stat_get_db_temp_bytes(PG_FUNCTION_ARGS)
+{
+	Oid		dbid = PG_GETARG_OID(0);
+	int64	result;
+	PgStat_StatDBEntry *dbentry;
+
+	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+		result = 0;
+	else
+		result = dbentry->n_temp_bytes;
+
+	PG_RETURN_INT64(result);
+}
+
+Datum
 pg_stat_get_db_conflict_tablespace(PG_FUNCTION_ARGS)
 {
 	Oid			dbid = PG_GETARG_OID(0);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 355c61a..df68a8d 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2633,6 +2633,10 @@ DATA(insert OID = 3070 (  pg_stat_get_db_conflict_all PGNSP PGUID 12 1 0 0 0 f f
 DESCR("statistics: recovery conflicts in database");
 DATA(insert OID = 3074 (  pg_stat_get_db_stat_reset_time PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 1184 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_stat_reset_time _null_ _null_ _null_ ));
 DESCR("statistics: last reset for a database");
+DATA(insert OID = 3079 (  pg_stat_get_db_temp_files PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_files _null_ _null_ _null_ ));
+DESCR("statistics: number of temporary files written");
+DATA(insert OID = 3080 (  pg_stat_get_db_temp_bytes PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 20 "26" _null_ _null_ _null_ _null_ pg_stat_get_db_temp_bytes _null_ _null_ _null_ ));
+DESCR("statistics: number of bytes in temporary files written");
 DATA(insert OID = 2769 ( pg_stat_get_bgwriter_timed_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_timed_checkpoints _null_ _null_ _null_ ));
 DESCR("statistics: number of timed checkpoints started by the bgwriter");
 DATA(insert OID = 2770 ( pg_stat_get_bgwriter_requested_checkpoints PGNSP PGUID 12 1 0 0 0 f f f t f s 0 0 20 "" _null_ _null_ _null_ _null_ pg_stat_get_bgwriter_requested_checkpoints _null_ _null_ _null_ ));
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index b8c6d82..df27d80 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -47,7 +47,8 @@ typedef enum StatMsgType
 	PGSTAT_MTYPE_BGWRITER,
 	PGSTAT_MTYPE_FUNCSTAT,
 	PGSTAT_MTYPE_FUNCPURGE,
-	PGSTAT_MTYPE_RECOVERYCONFLICT
+	PGSTAT_MTYPE_RECOVERYCONFLICT,
+	PGSTAT_MTYPE_TEMPFILE
 } StatMsgType;
 
 /* ----------
@@ -377,6 +378,18 @@ typedef struct PgStat_MsgRecoveryConflict
 } PgStat_MsgRecoveryConflict;
 
 /* ----------
+ * PgStat_MsgTempFile	Sent by the backend upon creating a temp file
+ * ----------
+ */
+typedef struct PgStat_MsgTempFile
+{
+	PgStat_MsgHdr m_hdr;
+
+	Oid			m_databaseid;
+	size_t		m_filesize;
+} PgStat_MsgTempFile;
+
+/* ----------
  * PgStat_FunctionCounts	The actual per-function counts kept by a backend
  *
  * This struct should contain only actual event counters, because we memcmp
@@ -507,9 +520,11 @@ typedef struct PgStat_StatDBEntry
 	PgStat_Counter n_conflict_snapshot;
 	PgStat_Counter n_conflict_bufferpin;
 	PgStat_Counter n_conflict_startup_deadlock;
+	PgStat_Counter n_temp_files;
+	PgStat_Counter n_temp_bytes;
+	
 	TimestampTz stat_reset_timestamp;
 
-
 	/*
 	 * tables and functions must be last in the struct, because we don't write
 	 * the pointers out to the stats file.
@@ -716,6 +731,7 @@ extern void pgstat_initialize(void);
 extern void pgstat_bestart(void);
 
 extern void pgstat_report_activity(const char *cmd_str);
+extern void pgstat_report_tempfile(size_t filesize);
 extern void pgstat_report_appname(const char *appname);
 extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
 extern void pgstat_report_waiting(bool waiting);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 454e1f9..c558066 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1296,7 +1296,7 @@ SELECT viewname, definition FROM pg_views WHERE schemaname <> 'information_schem
  pg_stat_all_indexes             | SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, c.relname, i.relname AS indexrelname, pg_stat_get_numscans(i.oid) AS idx_scan, pg_stat_get_tuples_returned(i.oid) AS idx_tup_read, pg_stat_get_tuples_fetched(i.oid) AS idx_tup_fetch FROM (((pg_class c JOIN pg_index x ON ((c.oid = x.indrelid))) JOIN pg_class i ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"]));
  pg_stat_all_tables              | SELECT c.oid AS relid, n.nspname AS schemaname, c.relname, pg_stat_get_numscans(c.oid) AS seq_scan, pg_stat_get_tuples_returned(c.oid) AS seq_tup_read, (sum(pg_stat_get_numscans(i.indexrelid)))::bigint AS idx_scan, ((sum(pg_stat_get_tuples_fetched(i.indexrelid)))::bigint + pg_stat_get_tuples_fetched(c.oid)) AS idx_tup_fetch, pg_stat_get_tuples_inserted(c.oid) AS n_tup_ins, pg_stat_get_tuples_updated(c.oid) AS n_tup_upd, pg_stat_get_tuples_deleted(c.oid) AS n_tup_del, pg_stat_get_tuples_hot_updated(c.oid) AS n_tup_hot_upd, pg_stat_get_live_tuples(c.oid) AS n_live_tup, pg_stat_get_dead_tuples(c.oid) AS n_dead_tup, pg_stat_get_last_vacuum_time(c.oid) AS last_vacuum, pg_stat_get_last_autovacuum_time(c.oid) AS last_autovacuum, pg_stat_get_last_analyze_time(c.oid) AS last_analyze, pg_stat_get_last_autoanalyze_time(c.oid) AS last_autoanalyze, pg_stat_get_vacuum_count(c.oid) AS vacuum_count, pg_stat_get_autovacuum_count(c.oid) AS autovacuum_count, pg_stat_get_analyze_count(c.oid) AS analyze_count, pg_stat_get_autoanalyze_count(c.oid) AS autoanalyze_count FROM ((pg_class c LEFT JOIN pg_index i ON ((c.oid = i.indrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"])) GROUP BY c.oid, n.nspname, c.relname;
  pg_stat_bgwriter                | SELECT pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints_timed, pg_stat_get_bgwriter_requested_checkpoints() AS checkpoints_req, pg_stat_get_bgwriter_buf_written_checkpoints() AS buffers_checkpoint, pg_stat_get_bgwriter_buf_written_clean() AS buffers_clean, pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean, pg_stat_get_buf_written_backend() AS buffers_backend, pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync, pg_stat_get_buf_alloc() AS buffers_alloc, pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
- pg_stat_database                | SELECT d.oid AS datid, d.datname, pg_stat_get_db_numbackends(d.oid) AS numbackends, pg_stat_get_db_xact_commit(d.oid) AS xact_commit, pg_stat_get_db_xact_rollback(d.oid) AS xact_rollback, (pg_stat_get_db_blocks_fetched(d.oid) - pg_stat_get_db_blocks_hit(d.oid)) AS blks_read, pg_stat_get_db_blocks_hit(d.oid) AS blks_hit, pg_stat_get_db_tuples_returned(d.oid) AS tup_returned, pg_stat_get_db_tuples_fetched(d.oid) AS tup_fetched, pg_stat_get_db_tuples_inserted(d.oid) AS tup_inserted, pg_stat_get_db_tuples_updated(d.oid) AS tup_updated, pg_stat_get_db_tuples_deleted(d.oid) AS tup_deleted, pg_stat_get_db_conflict_all(d.oid) AS conflicts, pg_stat_get_db_stat_reset_time(d.oid) AS stats_reset FROM pg_database d;
+ pg_stat_database                | SELECT d.oid AS datid, d.datname, pg_stat_get_db_numbackends(d.oid) AS numbackends, pg_stat_get_db_xact_commit(d.oid) AS xact_commit, pg_stat_get_db_xact_rollback(d.oid) AS xact_rollback, (pg_stat_get_db_blocks_fetched(d.oid) - pg_stat_get_db_blocks_hit(d.oid)) AS blks_read, pg_stat_get_db_blocks_hit(d.oid) AS blks_hit, pg_stat_get_db_tuples_returned(d.oid) AS tup_returned, pg_stat_get_db_tuples_fetched(d.oid) AS tup_fetched, pg_stat_get_db_tuples_inserted(d.oid) AS tup_inserted, pg_stat_get_db_tuples_updated(d.oid) AS tup_updated, pg_stat_get_db_tuples_deleted(d.oid) AS tup_deleted, pg_stat_get_db_conflict_all(d.oid) AS conflicts, pg_stat_get_db_temp_files(d.oid) AS temp_files, pg_stat_get_db_temp_bytes(d.oid) AS temp_bytes, pg_stat_get_db_stat_reset_time(d.oid) AS stats_reset FROM pg_database d;
  pg_stat_database_conflicts      | SELECT d.oid AS datid, d.datname, pg_stat_get_db_conflict_tablespace(d.oid) AS confl_tablespace, pg_stat_get_db_conflict_lock(d.oid) AS confl_lock, pg_stat_get_db_conflict_snapshot(d.oid) AS confl_snapshot, pg_stat_get_db_conflict_bufferpin(d.oid) AS confl_bufferpin, pg_stat_get_db_conflict_startup_deadlock(d.oid) AS confl_deadlock FROM pg_database d;
  pg_stat_replication             | SELECT s.procpid, s.usesysid, u.rolname AS usename, s.application_name, s.client_addr, s.client_hostname, s.client_port, s.backend_start, w.state, w.sent_location, w.write_location, w.flush_location, w.replay_location, w.sync_priority, w.sync_state FROM pg_stat_get_activity(NULL::integer) s(datid, procpid, usesysid, application_name, current_query, waiting, xact_start, query_start, backend_start, client_addr, client_hostname, client_port), pg_authid u, pg_stat_get_wal_senders() w(procpid, state, sent_location, write_location, flush_location, replay_location, sync_priority, sync_state) WHERE ((s.usesysid = u.oid) AND (s.procpid = w.procpid));
  pg_stat_sys_indexes             | SELECT pg_stat_all_indexes.relid, pg_stat_all_indexes.indexrelid, pg_stat_all_indexes.schemaname, pg_stat_all_indexes.relname, pg_stat_all_indexes.indexrelname, pg_stat_all_indexes.idx_scan, pg_stat_all_indexes.idx_tup_read, pg_stat_all_indexes.idx_tup_fetch FROM pg_stat_all_indexes WHERE ((pg_stat_all_indexes.schemaname = ANY (ARRAY['pg_catalog'::name, 'information_schema'::name])) OR (pg_stat_all_indexes.schemaname ~ '^pg_toast'::text));
#7Magnus Hagander
magnus@hagander.net
In reply to: Tomas Vondra (#6)
Re: PATCH: tracking temp files in pg_stat_database

On Tue, Jan 17, 2012 at 21:39, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20.12.2011 19:59, Tomas Vondra wrote:

On 20.12.2011 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

I've added the docs (see the attachment) and rebased to current head.

Tomas

Fixed a failing regression test (check of pg_stat_database structure).

I'm wondering if this (and also my deadlocks stats patch that's int he
queue) should instead of inventing new pgstats messages, add fields to
the tabstat message. It sounds like that one is just for tables, but
it's already the one collecting info about commits and rollbacks, and
it's already sent on every commit.

Adding two fields to that one would add some extra bytes on every
send, but I wonder if that woudl ever affect performance, given the
total size of the packet? And it would certainly be lower overhead in
the cases that there *have* been temp tables used.

Thoughts?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#8Tomas Vondra
tv@fuzzy.cz
In reply to: Magnus Hagander (#7)
Re: PATCH: tracking temp files in pg_stat_database

On 20 Leden 2012, 13:23, Magnus Hagander wrote:

On Tue, Jan 17, 2012 at 21:39, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20.12.2011 19:59, Tomas Vondra wrote:

On 20.12.2011 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable
at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

I've added the docs (see the attachment) and rebased to current head.

Tomas

Fixed a failing regression test (check of pg_stat_database structure).

I'm wondering if this (and also my deadlocks stats patch that's int he
queue) should instead of inventing new pgstats messages, add fields to
the tabstat message. It sounds like that one is just for tables, but
it's already the one collecting info about commits and rollbacks, and
it's already sent on every commit.

Hmmm, I'm not against that, but I'd recommend changing the message name to
something that reflects the reality. If it's not just about table
statistics, it should not be named 'tabstats' IMHO. Or maybe split that
into two messages, both sent at the commit time.

I do like the idea of not sending the message for each temp file, though.
One thing that worries me are long running transactions (think about a
batch process that runs for several hours within a single transaction). By
sending the data only at the commit, such transactions would not be
accounted properly. So I'd suggest sending the message either at commit
time or after collecting enough data (increment a counter whenever the
struct is updated and send a message when the counter >= N for a
reasonable value of N, say 20). But maybe it already works that way - I
haven't checked the current 'tabstat' implementation.

Adding two fields to that one would add some extra bytes on every
send, but I wonder if that woudl ever affect performance, given the
total size of the packet? And it would certainly be lower overhead in
the cases that there *have* been temp tables used.

It's not about temp tables, it's about temp files. Which IMHO implies that
there would be exactly 0.000001% performance difference because temporary
files are quite expensive.

Tomas

#9Magnus Hagander
magnus@hagander.net
In reply to: Tomas Vondra (#8)
Re: PATCH: tracking temp files in pg_stat_database

On Sat, Jan 21, 2012 at 18:02, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20 Leden 2012, 13:23, Magnus Hagander wrote:

On Tue, Jan 17, 2012 at 21:39, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20.12.2011 19:59, Tomas Vondra wrote:

On 20.12.2011 11:20, Magnus Hagander wrote:

2011/12/20 Tomas Vondra <tv@fuzzy.cz>:

I haven't updated the docs yet - let's see if the patch is acceptable
at
all first.

Again, without having reviewed the code, this looks like a feature
we'd want, so please add some docs, and then submit it for the next
commitfest!

I've added the docs (see the attachment) and rebased to current head.

Tomas

Fixed a failing regression test (check of pg_stat_database structure).

I'm wondering if this (and also my deadlocks stats patch that's int he
queue) should instead of inventing new pgstats messages, add fields to
the tabstat message. It sounds like that one is just for tables, but
it's already the one collecting info about commits and rollbacks, and
it's already sent on every commit.

Hmmm, I'm not against that, but I'd recommend changing the message name to
something that reflects the reality. If it's not just about table
statistics, it should not be named 'tabstats' IMHO. Or maybe split that
into two messages, both sent at the commit time.

Yes, renaming it might be a good idea. If you split it into two
messages that would defeat much of the point.

Though I just looked at the tabstat code again, and we already split
that message up at regular intervals. Which would make it quite weird
to have global counters in it as well.

But instead of there, perhaps we need a general "non table, but more
than one type of data" message sent out at the same time. There is
other stuff in the queue for it.

I'm not convinced either way - I'm not against the original way in
your patch either. I just wanted to throw the idea out there, and was
hoping somebody else would have an opinion too :-)

I do like the idea of not sending the message for each temp file, though.
One thing that worries me are long running transactions (think about a
batch process that runs for several hours within a single transaction). By
sending the data only at the commit, such transactions would not be
accounted properly. So I'd suggest sending the message either at commit

By that argument they are *already* not accounted properly, because
the "number of rows" and those counters are wrong. By sending the temp
file data in the middle of the transaction, you won't be able to
correlate those numbers with the temp file usage.

I'm not saying the other usecase isn't more common, but whichever way
you do it, it's going to get inconsistent with *something*.

time or after collecting enough data (increment a counter whenever the
struct is updated and send a message when the counter >= N for a
reasonable value of N, say 20). But maybe it already works that way - I
haven't checked the current 'tabstat' implementation.

No, tabstat is sent at transaction end only.

Adding two fields to that one would add some extra bytes on every
send, but I wonder if that woudl ever affect performance, given the
total size of the packet? And it would certainly be lower overhead in
the cases that there *have* been temp tables used.

It's not about temp tables, it's about temp files. Which IMHO implies that
there would be exactly 0.000001% performance difference because temporary
files are quite expensive.

Bad choice of words, I meant temp files, and was thinking temp files
the whole way :-)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#10Tomas Vondra
tv@fuzzy.cz
In reply to: Magnus Hagander (#9)
Re: PATCH: tracking temp files in pg_stat_database

On 21 Leden 2012, 18:13, Magnus Hagander wrote:

On Sat, Jan 21, 2012 at 18:02, Tomas Vondra <tv@fuzzy.cz> wrote:

On 20 Leden 2012, 13:23, Magnus Hagander wrote:

I'm wondering if this (and also my deadlocks stats patch that's int he
queue) should instead of inventing new pgstats messages, add fields to
the tabstat message. It sounds like that one is just for tables, but
it's already the one collecting info about commits and rollbacks, and
it's already sent on every commit.

Hmmm, I'm not against that, but I'd recommend changing the message name
to
something that reflects the reality. If it's not just about table
statistics, it should not be named 'tabstats' IMHO. Or maybe split that
into two messages, both sent at the commit time.

Yes, renaming it might be a good idea. If you split it into two
messages that would defeat much of the point.

Not really, if combined with the 'send after each N updates or at the
transaction end' because then those parts might be sent independently
(thus not transmitting the whole message).

Though I just looked at the tabstat code again, and we already split
that message up at regular intervals. Which would make it quite weird
to have global counters in it as well.

But instead of there, perhaps we need a general "non table, but more
than one type of data" message sent out at the same time. There is
other stuff in the queue for it.

I'm not convinced either way - I'm not against the original way in
your patch either. I just wanted to throw the idea out there, and was
hoping somebody else would have an opinion too :-)

Let's make that in a separate patch. It seems like a good thing to do, but
I don't think it should be mixed with this patch.

I do like the idea of not sending the message for each temp file,
though.
One thing that worries me are long running transactions (think about a
batch process that runs for several hours within a single transaction).
By
sending the data only at the commit, such transactions would not be
accounted properly. So I'd suggest sending the message either at commit

By that argument they are *already* not accounted properly, because
the "number of rows" and those counters are wrong. By sending the temp
file data in the middle of the transaction, you won't be able to
correlate those numbers with the temp file usage.

I'm not saying the other usecase isn't more common, but whichever way
you do it, it's going to get inconsistent with *something*.

The question is whether the patch should be modified to send the message
only at commit (i.e. just like tabstats) or if it can remain the way it is
now. And maybe we could change the way all those messages are sent (e.g.
to the regular submits I've proposed in the previous post) to make them
more consistent.

I'm not asking for 100% consistency. What I don't like is a batch process
consuming resources but not reflected in the stats until the final commit.
With a huge spike in the stats right after that, although the activity was
actually spread over several hours.

Tomas

#11Magnus Hagander
magnus@hagander.net
In reply to: Tomas Vondra (#10)
Re: PATCH: tracking temp files in pg_stat_database

On Sat, Jan 21, 2012 at 20:12, Tomas Vondra <tv@fuzzy.cz> wrote:

On 21 Leden 2012, 18:13, Magnus Hagander wrote:

On Sat, Jan 21, 2012 at 18:02, Tomas Vondra <tv@fuzzy.cz> wrote:
Though I just looked at the tabstat code again, and we already split
that message up at regular intervals. Which would make it quite weird
to have global counters in it as well.

But instead of there, perhaps we need a general "non table, but more
than one type of data" message sent out at the same time. There is
other stuff in the queue for it.

I'm not convinced either way - I'm not against the original way in
your patch either. I just wanted to throw the idea out there, and was
hoping somebody else would have an opinion too :-)

Let's make that in a separate patch. It seems like a good thing to do, but
I don't think it should be mixed with this patch.

Yeah, I'm not sure we even want to do that yet, when we go down this
road. We might eventually, of course.

I do like the idea of not sending the message for each temp file,
though.
One thing that worries me are long running transactions (think about a
batch process that runs for several hours within a single transaction).
By
sending the data only at the commit, such transactions would not be
accounted properly. So I'd suggest sending the message either at commit

By that argument they are *already* not accounted properly, because
the "number of rows" and those counters are wrong. By sending the temp
file data in the middle of the transaction, you won't be able to
correlate those numbers with the temp file usage.

I'm not saying the other usecase isn't more common, but whichever way
you do it, it's going to get inconsistent with *something*.

The question is whether the patch should be modified to send the message
only at commit (i.e. just like tabstats) or if it can remain the way it is
now. And maybe we could change the way all those messages are sent (e.g.
to the regular submits I've proposed in the previous post) to make them
more consistent.

I'm not asking for 100% consistency. What I don't like is a batch process
consuming resources but not reflected in the stats until the final commit.
With a huge spike in the stats right after that, although the activity was
actually spread over several hours.

Yeah, having thought about it some more, I think sending them when
they happen is a better idea. (It's obviously still only going to send
them when the file is closed, but that's as close as we can reasonably
get).

I've cleaned up the patch a bit and applied it - thanks!

Other than cosmetic, I changed the text in the description around a
bit (sol if that's worse now, that's purely my fault) and added docs
to the section about pg_stat_database that you'd forgotten.

Also, I'd like to point you in the direction of the
src/include/catalog/find_unused_oids and duplicate_oids scripts. The
oids you'd picked conflicted with the pg_extension catalog from a year
ago. Easy enough to fix, and there are often conflicts with more
recent oids that the committer has to deal with anyway, but cleaning
those up before submission always helps a little bit. For the next one
:-)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#12Tomas Vondra
tv@fuzzy.cz
In reply to: Magnus Hagander (#11)
Re: PATCH: tracking temp files in pg_stat_database

On 26 Leden 2012, 14:44, Magnus Hagander wrote:

On Sat, Jan 21, 2012 at 20:12, Tomas Vondra <tv@fuzzy.cz> wrote:

On 21 Leden 2012, 18:13, Magnus Hagander wrote:

On Sat, Jan 21, 2012 at 18:02, Tomas Vondra <tv@fuzzy.cz> wrote:
Though I just looked at the tabstat code again, and we already split
that message up at regular intervals. Which would make it quite weird
to have global counters in it as well.

But instead of there, perhaps we need a general "non table, but more
than one type of data" message sent out at the same time. There is
other stuff in the queue for it.

I'm not convinced either way - I'm not against the original way in
your patch either. I just wanted to throw the idea out there, and was
hoping somebody else would have an opinion too :-)

Let's make that in a separate patch. It seems like a good thing to do,
but
I don't think it should be mixed with this patch.

Yeah, I'm not sure we even want to do that yet, when we go down this
road. We might eventually, of course.

Yes, that's one of the reasons why I suggested to do that in a separate
patch (that might be rejected if we find it's a bad idea).

I've cleaned up the patch a bit and applied it - thanks!

Other than cosmetic, I changed the text in the description around a
bit (sol if that's worse now, that's purely my fault) and added docs
to the section about pg_stat_database that you'd forgotten.

Great. Thanks for the fixes.

Also, I'd like to point you in the direction of the
src/include/catalog/find_unused_oids and duplicate_oids scripts. The
oids you'd picked conflicted with the pg_extension catalog from a year
ago. Easy enough to fix, and there are often conflicts with more
recent oids that the committer has to deal with anyway, but cleaning
those up before submission always helps a little bit. For the next one
:-)

Aha! I've been wondering how the commiters identify duplicate oids etc.
but I was unaware of those scripts.

Tomas