Skip site navigation (1) Skip section navigation (2)

pgsql: Improve coding around the fsync request queue.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Improve coding around the fsync request queue.
Date: 2012-07-17 20:57:38
Message-ID: E1SrEpu-0007Fl-Im@gemulon.postgresql.org (view raw or flat)
Thread:
Lists: pgsql-committers
Improve coding around the fsync request queue.

In all branches back to 8.3, this patch fixes a questionable assumption in
CompactCheckpointerRequestQueue/CompactBgwriterRequestQueue that there are
no uninitialized pad bytes in the request queue structs.  This would only
cause trouble if (a) there were such pad bytes, which could happen in 8.4
and up if the compiler makes enum ForkNumber narrower than 32 bits, but
otherwise would require not-currently-planned changes in the widths of
other typedefs; and (b) the kernel has not uniformly initialized the
contents of shared memory to zeroes.  Still, it seems a tad risky, and we
can easily remove any risk by pre-zeroing the request array for ourselves.
In addition to that, we need to establish a coding rule that struct
RelFileNode can't contain any padding bytes, since such structs are copied
into the request array verbatim.  (There are other places that are assuming
this anyway, it turns out.)

In 9.1 and up, the risk was a bit larger because we were also effectively
assuming that struct RelFileNodeBackend contained no pad bytes, and with
fields of different types in there, that would be much easier to break.
However, there is no good reason to ever transmit fsync or delete requests
for temp files to the bgwriter/checkpointer, so we can revert the request
structs to plain RelFileNode, getting rid of the padding risk and saving
some marginal number of bytes and cycles in fsync queue manipulation while
we are at it.  The savings might be more than marginal during deletion of
a temp relation, because the old code transmitted an entirely useless but
nonetheless expensive-to-process ForgetRelationFsync request to the
background process, and also had the background process perform the file
deletion even though that can safely be done immediately.

In addition, make some cleanup of nearby comments and small improvements to
the code in CompactCheckpointerRequestQueue/CompactBgwriterRequestQueue.

Branch
------
REL9_2_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/4abcce8cabe3845140f9a7cd8470e27e44f208bb

Modified Files
--------------
src/backend/postmaster/checkpointer.c |   51 ++++++++++++++------
src/backend/storage/buffer/bufmgr.c   |    4 +-
src/backend/storage/smgr/md.c         |   87 ++++++++++++++++++++-------------
src/include/postmaster/bgwriter.h     |    2 +-
src/include/storage/relfilenode.h     |   16 +++++-
src/include/storage/smgr.h            |    7 +--
6 files changed, 109 insertions(+), 58 deletions(-)

pgsql-committers by date

Next:From: Tom LaneDate: 2012-07-18 05:13:38
Subject: pgsql: Improve pg_upgrade's load_directory() function.
Previous:From: Peter EisentrautDate: 2012-07-17 20:28:42
Subject: pgsql: PL/Python: Remove PLy_result_ass_item

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group