(2012/11/27 7:42), Alvaro Herrera wrote:
Satoshi Nagayasu escribió:

I attached the latest one, which splits the reset_time
for bgwriter and walwriter, and provides new system view,
called pg_stat_walwriter, to show the dirty write counter
and the reset time.

Thanks.  I gave this a look and I have a couple of comments:

1. The counter is called dirty_write.  I imagine that this comes
directly from the name of the nearby DTrace probe,
TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_START.  That probe comes from
email 494c1565.3060...@sun.com committed in 4ee79fd20d9a.  But there
wasn't much discussion about the name back then.  Maybe that was fine at
the time because it was pretty much an internal matter, being so deep in
the code.  But we're now going to expose it to regular users, so we'd
better choose a very good name because we're going to be stuck with it
for a very long time.  And the name "dirty" doesn't ring with me too
well; what matters here is not that we're writing a buffer that is
dirty, but the fact that we're writing while holding the WalInsertLock,
so the name should convey the fact that this is a "locked" write or
something like that.  Or at least that's how I see the issue.  Note the
documentation wording:

+      <entry><structfield>dirty_writes</></entry>
+      <entry><type>bigint</type></entry>
+      <entry>Number of dirty writes, which means flushing wal buffers
+       because of its full.</entry>

Yes, "dirty_writes" came from the probe name, and if it needs to be changed, "buffers_flush" would make sense for me in this situation, because this counter is intended to show WAL writes due to "wal buffer full".

2. Should we put bgwriter and walwriter data in separate structs?  I
don't think this is necessary, but maybe someone opines differently?

I tried to minimize an impact of this patch, but if I can change this struct, yes, I'd like to split into two structs.

3.
+/*
+ * WalWriter global statistics counter.
+ * Despite its name, this counter is actually used not only in walwriter,
+ * but also in each backend process to sum up xlog dirty writes.
+ * Those processes would increment this counter in each XLogWrite call,
+ * then send it to the stat collector process.
+ */
+PgStat_MsgWalWriter WalWriterStats;

Maybe we should use a different name for the struct, to avoid having to
excuse ourselves for the name being wrong ...

Ok. How about WalBufferStats? I think this name could be accepted in both the wal writer and each backend process.

--
Satoshi Nagayasu <sn...@uptime.jp>
Uptime Technologies, LLC. http://www.uptime.jp


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to