Search Postgresql Archives

Lockless pg_buffercache

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I want to use the pg_buffercache contrib module for monitoring our
server. It takes a lock on all buffers and then on each buffer header
in order to get a consistent picture of the buffers. I would be
running the function provided by the module once every 5 minutes.

I'm worrying about the performance hit of that - a comment in the code
says it's horrible for concurrency. Additionally, as I don't use this
for debugging, but just for monitoring, I don't need a 100% consistent
picture, just rough numbers how much of the buffer cache is used for
what relation.

Does removing all locking as in the attached patch have any negative
impact other than the non-consistency of the results?

Thanks

Markus
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index b1c3fbc..fe5c880 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -108,23 +108,11 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		MemoryContextSwitchTo(oldcontext);
 
 		/*
-		 * To get a consistent picture of the buffer state, we must lock all
-		 * partitions of the buffer map.  Needless to say, this is horrible
-		 * for concurrency.  Must grab locks in increasing order to avoid
-		 * possible deadlocks.
-		 */
-		for (i = 0; i < NUM_BUFFER_PARTITIONS; i++)
-			LWLockAcquire(FirstBufMappingLock + i, LW_SHARED);
-
-		/*
 		 * Scan though all the buffers, saving the relevant fields in the
 		 * fctx->record structure.
 		 */
 		for (i = 0, bufHdr = BufferDescriptors; i < NBuffers; i++, bufHdr++)
 		{
-			/* Lock each buffer header before inspecting. */
-			LockBufHdr(bufHdr);
-
 			fctx->record[i].bufferid = BufferDescriptorGetBuffer(bufHdr);
 			fctx->record[i].relfilenode = bufHdr->tag.rnode.relNode;
 			fctx->record[i].reltablespace = bufHdr->tag.rnode.spcNode;
@@ -142,19 +130,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 				fctx->record[i].isvalid = true;
 			else
 				fctx->record[i].isvalid = false;
-
-			UnlockBufHdr(bufHdr);
 		}
-
-		/*
-		 * And release locks.  We do this in reverse order for two reasons:
-		 * (1) Anyone else who needs more than one of the locks will be trying
-		 * to lock them in increasing order; we don't want to release the
-		 * other process until it can get all the locks it needs. (2) This
-		 * avoids O(N^2) behavior inside LWLockRelease.
-		 */
-		for (i = NUM_BUFFER_PARTITIONS; --i >= 0;)
-			LWLockRelease(FirstBufMappingLock + i);
 	}
 
 	funcctx = SRF_PERCALL_SETUP();
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to majordomo@xxxxxxxxxxxxxx so that your
       message can get through to the mailing list cleanly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Postgresql Jobs]     [Postgresql Admin]     [Postgresql Performance]     [Linux Clusters]     [PHP Home]     [PHP on Windows]     [Kernel Newbies]     [PHP Classes]     [PHP Books]     [PHP Databases]     [Postgresql & PHP]     [Yosemite]
  Powered by Linux