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