Code review for dtrace probes added (so far) to 8.4. Adjust placement of

some bufmgr probes, take out redundant and memory-leak-inducing path arguments
to smgr__md__read__done and smgr__md__write__done, fix bogus attempt to
recalculate space used in sort__done, clean up formatting in places where
I'm not sure pgindent will do a nice job by itself.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3539e9a946e6e1eb6c2b4c63c6b60c4af5a75fac..d1ea169d213d2d96511fe67f070f167f7559deec 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6384,10 +6384,11 @@ CreateCheckPoint(int flags)
        if (log_checkpoints)
                LogCheckpointEnd(false);
 
-        TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
-                                NBuffers, CheckpointStats.ckpt_segs_added,
-                                CheckpointStats.ckpt_segs_removed,
-                                CheckpointStats.ckpt_segs_recycled);
+       TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
+                                                                        NBuffers,
+                                                                        CheckpointStats.ckpt_segs_added,
+                                                                        CheckpointStats.ckpt_segs_removed,
+                                                                        CheckpointStats.ckpt_segs_recycled);
 
        LWLockRelease(CheckpointLock);
 }

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index bd053d503de04f98d202fb2e386529be251bc9fc..b9c030b7aa4999093e5bdab4f8831951e01a0ae1 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -260,7 +260,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
        if (isExtend)
                blockNum = smgrnblocks(smgr, forkNum);
 
-       TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, smgr->smgr_rnode.spcNode, smgr->smgr_rnode.dbNode, smgr->smgr_rnode.relNode, isLocalBuf);
+       TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum,
+                                                                          smgr->smgr_rnode.spcNode,
+                                                                          smgr->smgr_rnode.dbNode,
+                                                                          smgr->smgr_rnode.relNode,
+                                                                          isLocalBuf);
 
        if (isLocalBuf)
        {
@@ -269,11 +273,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
                if (found)
                {
                        LocalBufferHitCount++;
-                       TRACE_POSTGRESQL_BUFFER_HIT(true); /* true == local buffer */
+                       TRACE_POSTGRESQL_BUFFER_HIT(true);              /* true = local buffer */
                }
                else
                {
-                       TRACE_POSTGRESQL_BUFFER_MISS(true); /* ditto */
+                       TRACE_POSTGRESQL_BUFFER_MISS(true);             /* ditto */
                }
        }
        else
@@ -288,11 +292,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
                if (found)
                {
                        BufferHitCount++;
-                       TRACE_POSTGRESQL_BUFFER_HIT(false); /* false != local buffer */
+                       TRACE_POSTGRESQL_BUFFER_HIT(false);             /* false = shared buffer */
                }
                else
                {
-                       TRACE_POSTGRESQL_BUFFER_MISS(false); /* ditto */
+                       TRACE_POSTGRESQL_BUFFER_MISS(false);    /* ditto */
                }
        }
 
@@ -310,9 +314,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
                                VacuumCostBalance += VacuumCostPageHit;
 
                        TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
-                               smgr->smgr_rnode.spcNode,
-                               smgr->smgr_rnode.dbNode,
-                               smgr->smgr_rnode.relNode, isLocalBuf, found);
+                                                                                         smgr->smgr_rnode.spcNode,
+                                                                                         smgr->smgr_rnode.dbNode,
+                                                                                         smgr->smgr_rnode.relNode,
+                                                                                         isLocalBuf,
+                                                                                         found);
 
                        return BufferDescriptorGetBuffer(bufHdr);
                }
@@ -437,8 +443,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
                VacuumCostBalance += VacuumCostPageMiss;
 
        TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
-                       smgr->smgr_rnode.spcNode, smgr->smgr_rnode.dbNode,
-                       smgr->smgr_rnode.relNode, isLocalBuf, found);
+                                                                         smgr->smgr_rnode.spcNode,
+                                                                         smgr->smgr_rnode.dbNode,
+                                                                         smgr->smgr_rnode.relNode,
+                                                                         isLocalBuf,
+                                                                         found);
 
        return BufferDescriptorGetBuffer(bufHdr);
 }
@@ -582,11 +591,6 @@ BufferAlloc(SMgrRelation smgr, ForkNumber forkNum,
                         * happens to be trying to split the page the first one got from
                         * StrategyGetBuffer.)
                         */
-
-                        TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_START(forkNum,
-                         blockNum, smgr->smgr_rnode.spcNode,
-                         smgr->smgr_rnode.dbNode, smgr->smgr_rnode.relNode);
-
                        if (LWLockConditionalAcquire(buf->content_lock, LW_SHARED))
                        {
                                /*
@@ -607,13 +611,18 @@ BufferAlloc(SMgrRelation smgr, ForkNumber forkNum,
                                }
 
                                /* OK, do the I/O */
+                               TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_START(forkNum, blockNum,
+                                                                                                 smgr->smgr_rnode.spcNode,
+                                                                                                 smgr->smgr_rnode.dbNode,
+                                                                                                 smgr->smgr_rnode.relNode);
+
                                FlushBuffer(buf, NULL);
                                LWLockRelease(buf->content_lock);
 
-                                TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_DONE(
-                                  forkNum, blockNum, smgr->smgr_rnode.spcNode,
-                                  smgr->smgr_rnode.dbNode,
-                                 smgr->smgr_rnode.relNode);
+                               TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY_DONE(forkNum, blockNum,
+                                                                                                        smgr->smgr_rnode.spcNode,
+                                                                                                        smgr->smgr_rnode.dbNode,
+                                                                                                        smgr->smgr_rnode.relNode);
                        }
                        else
                        {
@@ -1235,13 +1244,13 @@ BufferSync(int flags)
                        buf_id = 0;
        }
 
-       TRACE_POSTGRESQL_BUFFER_SYNC_DONE(NBuffers, num_written, num_to_write);
-
        /*
         * Update checkpoint statistics. As noted above, this doesn't include
         * buffers written by other backends or bgwriter scan.
         */
        CheckpointStats.ckpt_bufs_written += num_written;
+
+       TRACE_POSTGRESQL_BUFFER_SYNC_DONE(NBuffers, num_written, num_to_write);
 }
 
 /*
@@ -1852,14 +1861,14 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
        errcontext.previous = error_context_stack;
        error_context_stack = &errcontext;
 
+       TRACE_POSTGRESQL_BUFFER_FLUSH_START(reln->smgr_rnode.spcNode,
+                                                                               reln->smgr_rnode.dbNode,
+                                                                               reln->smgr_rnode.relNode);
+
        /* Find smgr relation for buffer */
        if (reln == NULL)
                reln = smgropen(buf->tag.rnode);
 
-       TRACE_POSTGRESQL_BUFFER_FLUSH_START(reln->smgr_rnode.spcNode,
-                reln->smgr_rnode.dbNode,
-                reln->smgr_rnode.relNode);
-
        /*
         * Force XLOG flush up to buffer's LSN.  This implements the basic WAL
         * rule that log updates must hit disk before any of the data-file changes
@@ -1887,15 +1896,16 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
 
        BufferFlushCount++;
 
-       TRACE_POSTGRESQL_BUFFER_FLUSH_DONE(reln->smgr_rnode.spcNode,
-                reln->smgr_rnode.dbNode, reln->smgr_rnode.relNode);
-
        /*
         * Mark the buffer as clean (unless BM_JUST_DIRTIED has become set) and
         * end the io_in_progress state.
         */
        TerminateBufferIO(buf, true, 0);
 
+       TRACE_POSTGRESQL_BUFFER_FLUSH_DONE(reln->smgr_rnode.spcNode,
+                                                                          reln->smgr_rnode.dbNode,
+                                                                          reln->smgr_rnode.relNode);
+
        /* Pop the error context stack */
        error_context_stack = errcontext.previous;
 }

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 1b407c923d04253d89ffe4764b7a2b740be02010..777cdcefa2c1b52f4e53b68dd96e573315bfdf9a 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -581,7 +581,10 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        int                     nbytes;
        MdfdVec    *v;
 
-       TRACE_POSTGRESQL_SMGR_MD_READ_START(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode, reln->smgr_rnode.relNode);
+       TRACE_POSTGRESQL_SMGR_MD_READ_START(forknum, blocknum,
+                                                                               reln->smgr_rnode.spcNode,
+                                                                               reln->smgr_rnode.dbNode,
+                                                                               reln->smgr_rnode.relNode);
 
        v = _mdfd_getseg(reln, forknum, blocknum, false, EXTENSION_FAIL);
 
@@ -596,7 +599,12 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 
        nbytes = FileRead(v->mdfd_vfd, buffer, BLCKSZ);
 
-       TRACE_POSTGRESQL_SMGR_MD_READ_DONE(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode, reln->smgr_rnode.relNode, relpath(reln->smgr_rnode, forknum), nbytes, BLCKSZ);
+       TRACE_POSTGRESQL_SMGR_MD_READ_DONE(forknum, blocknum,
+                                                                          reln->smgr_rnode.spcNode,
+                                                                          reln->smgr_rnode.dbNode,
+                                                                          reln->smgr_rnode.relNode,
+                                                                          nbytes,
+                                                                          BLCKSZ);
 
        if (nbytes != BLCKSZ)
        {
@@ -645,7 +653,10 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        Assert(blocknum < mdnblocks(reln, forknum));
 #endif
 
-       TRACE_POSTGRESQL_SMGR_MD_WRITE_START(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode, reln->smgr_rnode.relNode);
+       TRACE_POSTGRESQL_SMGR_MD_WRITE_START(forknum, blocknum,
+                                                                                reln->smgr_rnode.spcNode,
+                                                                                reln->smgr_rnode.dbNode,
+                                                                                reln->smgr_rnode.relNode);
 
        v = _mdfd_getseg(reln, forknum, blocknum, isTemp, EXTENSION_FAIL);
 
@@ -660,7 +671,12 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 
        nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ);
 
-       TRACE_POSTGRESQL_SMGR_MD_WRITE_DONE(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode, reln->smgr_rnode.relNode, relpath(reln->smgr_rnode, forknum), nbytes, BLCKSZ);
+       TRACE_POSTGRESQL_SMGR_MD_WRITE_DONE(forknum, blocknum,
+                                                                               reln->smgr_rnode.spcNode,
+                                                                               reln->smgr_rnode.dbNode,
+                                                                               reln->smgr_rnode.relNode,
+                                                                               nbytes,
+                                                                               BLCKSZ);
 
        if (nbytes != BLCKSZ)
        {

diff --git a/src/backend/utils/probes.d b/src/backend/utils/probes.d
index f68a7d2157528b74bb0dfd4ca8b03a958058505b..078b81655e6f4e234dd7a111caaef66244fb1689 100644 (file)
--- a/src/backend/utils/probes.d
+++ b/src/backend/utils/probes.d
@@ -8,7 +8,12 @@
  */
 
 
-/* typedefs used in PostgreSQL */
+/*
+ * Typedefs used in PostgreSQL.
+ *
+ * NOTE: Do not use system-provided typedefs (e.g. uintptr_t, uint32_t, etc)
+ * in probe definitions, as they cause compilation errors on Mac OS X 10.5.
+ */
 #define LocalTransactionId unsigned int
 #define LWLockId int
 #define LWLockMode int
@@ -20,10 +25,6 @@
 
 provider postgresql {
 
-       /* 
-        * Note: Do not use built-in typedefs (e.g. uintptr_t, uint32_t, etc)            *       as they cause compilation errors in Mac OS X 10.5.  
-        */
-         
        probe transaction__start(LocalTransactionId);
        probe transaction__commit(LocalTransactionId);
        probe transaction__abort(LocalTransactionId);
@@ -51,7 +52,7 @@ provider postgresql {
        probe statement__status(const char *);
 
        probe sort__start(int, bool, int, int, bool);
-       probe sort__done(unsigned long, long);
+       probe sort__done(bool, long);
 
        probe buffer__read__start(ForkNumber, BlockNumber, Oid, Oid, Oid, bool);
        probe buffer__read__done(ForkNumber, BlockNumber, Oid, Oid, Oid, bool, bool);
@@ -83,9 +84,9 @@ provider postgresql {
        probe twophase__checkpoint__done();
 
        probe smgr__md__read__start(ForkNumber, BlockNumber, Oid, Oid, Oid);
-       probe smgr__md__read__done(ForkNumber, BlockNumber, Oid, Oid, Oid, const char *, int, int);
+       probe smgr__md__read__done(ForkNumber, BlockNumber, Oid, Oid, Oid, int, int);
        probe smgr__md__write__start(ForkNumber, BlockNumber, Oid, Oid, Oid);
-       probe smgr__md__write__done(ForkNumber, BlockNumber, Oid, Oid, Oid, const char *, int, int);
+       probe smgr__md__write__done(ForkNumber, BlockNumber, Oid, Oid, Oid, int, int);
 
        probe xlog__insert(unsigned char, unsigned char);
        probe xlog__switch();

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index b6d326ed9d4d157f7d0746180eb315cf8fcf2983..d31b0b393b4576b8e2640870b1127aa996be0bb4 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -117,15 +117,16 @@
 #include "utils/tuplesort.h"
 
 
+/* sort-type codes for sort__start probes */
+#define HEAP_SORT      0
+#define INDEX_SORT     1
+#define DATUM_SORT     2
+
 /* GUC variables */
 #ifdef TRACE_SORT
 bool           trace_sort = false;
 #endif
 
-#define HEAP_SORT      0
-#define INDEX_SORT     1
-#define DATUM_SORT     2
-
 #ifdef DEBUG_BOUNDED_SORT
 bool           optimize_bounded_sort = true;
 #endif
@@ -574,10 +575,14 @@ tuplesort_begin_heap(TupleDesc tupDesc,
                         nkeys, workMem, randomAccess ? 't' : 'f');
 #endif
 
-       TRACE_POSTGRESQL_SORT_START(HEAP_SORT, false, nkeys, workMem, randomAccess);
-
        state->nKeys = nkeys;
 
+       TRACE_POSTGRESQL_SORT_START(HEAP_SORT,
+                                                               false, /* no unique check */
+                                                               nkeys,
+                                                               workMem,
+                                                               randomAccess);
+
        state->comparetup = comparetup_heap;
        state->copytup = copytup_heap;
        state->writetup = writetup_heap;
@@ -642,7 +647,11 @@ tuplesort_begin_index_btree(Relation indexRel,
 
        state->nKeys = RelationGetNumberOfAttributes(indexRel);
 
-       TRACE_POSTGRESQL_SORT_START(INDEX_SORT, enforceUnique, state->nKeys, workMem, randomAccess);
+       TRACE_POSTGRESQL_SORT_START(INDEX_SORT,
+                                                               enforceUnique,
+                                                               state->nKeys,
+                                                               workMem,
+                                                               randomAccess);
 
        state->comparetup = comparetup_index_btree;
        state->copytup = copytup_index;
@@ -715,10 +724,14 @@ tuplesort_begin_datum(Oid datumType,
                         workMem, randomAccess ? 't' : 'f');
 #endif
 
-       TRACE_POSTGRESQL_SORT_START(DATUM_SORT, false, 1, workMem, randomAccess);
-
        state->nKeys = 1;                       /* always a one-column sort */
 
+       TRACE_POSTGRESQL_SORT_START(DATUM_SORT,
+                                                               false, /* no unique check */
+                                                               1,
+                                                               workMem,
+                                                               randomAccess);
+
        state->comparetup = comparetup_datum;
        state->copytup = copytup_datum;
        state->writetup = writetup_datum;
@@ -826,12 +839,16 @@ tuplesort_end(Tuplesortstate *state)
                        elog(LOG, "internal sort ended, %ld KB used: %s",
                                 spaceUsed, pg_rusage_show(&state->ru_start));
        }
-#endif
 
-       TRACE_POSTGRESQL_SORT_DONE(state->tapeset,
-                       (state->tapeset ? LogicalTapeSetBlocks(state->tapeset) :
-                       (state->allowedMem - state->availMem + 1023) / 1024));
+       TRACE_POSTGRESQL_SORT_DONE(state->tapeset != NULL, spaceUsed);
+#else
 
+       /*
+        * If you disabled TRACE_SORT, you can still probe sort__done, but
+        * you ain't getting space-used stats.
+        */
+       TRACE_POSTGRESQL_SORT_DONE(state->tapeset != NULL, 0L);
+#endif
 
        MemoryContextSwitchTo(oldcontext);