On 1/27/13 2:32 AM, Satoshi Nagayasu wrote:

This patch is intended to improve warning message at
AtEOXact_Buffers(), but I guess another function,
AtProcExit_Buffers(), needs to be modified as well. Right?

Yes, good catch. I've attached an updated patch that does the same sort of modification to that one too.

Then, I need some suggestion from hackers to continue this review.
How should I reproduce this message for review?
This is a debug warning message, so it's not easy for me
to reproduce this message.

What I was doing to test the code out is alter this check:

if (PrivateRefCount[i - 1] != 0)

To have the opposite logic:

if (PrivateRefCount[i - 1] = 0)

Then the patch fires on every buffer, all the time. You can still tell which are the real errors should you happen to get one, because the log entry shows the count. If it's 0, you know that's just a debugging message.

To keep the output from a test like that from being completely overwhelming, I also set:

shared_buffers = 16

Which is its minimum value.

--
Greg Smith   2ndQuadrant US    g...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com
diff --git a/src/backend/storage/buffer/bufmgr.c 
b/src/backend/storage/buffer/bufmgr.c
index 78a8adc..b6d4a74 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1713,12 +1713,20 @@ AtEOXact_Buffers(bool isCommit)
 #ifdef USE_ASSERT_CHECKING
        if (assert_enabled)
        {
-               int                     i;
+               Buffer                  i;
+               int                     RefCountErrors = 0;
 
-               for (i = 0; i < NBuffers; i++)
+               for (i = 1; i <= NBuffers; i++)
                {
-                       Assert(PrivateRefCount[i] == 0);
+                       if (PrivateRefCount[i - 1] != 0)
+                       {
+                               PrintBufferLeakWarning(i);      
+                               RefCountErrors++;
+                       }
                }
+               if (RefCountErrors > 0)
+                       elog(WARNING, "buffers with non-zero refcount is %d", 
RefCountErrors);
+               Assert(RefCountErrors == 0);
        }
 #endif
 
@@ -1753,12 +1761,21 @@ AtProcExit_Buffers(int code, Datum arg)
 #ifdef USE_ASSERT_CHECKING
        if (assert_enabled)
        {
-               int                     i;
+               Buffer                  i;
+               int                     RefCountErrors = 0;
 
-               for (i = 0; i < NBuffers; i++)
+               for (i = 1; i <= NBuffers; i++)
                {
-                       Assert(PrivateRefCount[i] == 0);
+                       if (PrivateRefCount[i - 1] != 0)
+                       {
+                               PrintBufferLeakWarning(i);      
+                               RefCountErrors++;
+                       }
                }
+               if (RefCountErrors > 0)
+                       elog(WARNING, "buffers with non-zero refcount is %d", 
RefCountErrors);
+               Assert(RefCountErrors == 0);
+
        }
 #endif
 
-- 
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