Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2)
Date
Msg-id 27074.1154644654@sss.pgh.pa.us
Whole thread Raw
In response to Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2)  (Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp>)
Responses Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2)
List pgsql-hackers
Katsuhiko Okano <okano.katsuhiko@oss.ntt.co.jp> writes:
>> (A) The algorithm which replaces a buffer is bad.
>> A time stamp does not become new until swapout completes 
>> the swapout page.
>> If access is during swap at other pages, the swapout page will be 
>> in the state where it is not used most,
>> It is again chosen as the page for swapout.
>> (When work load is high)

> The following is the patch.

I'm confused ... is this patch being proposed for inclusion?  I
understood your previous message to say that it didn't help much.

The patch is buggy as posted, because it will try to do this:         if (shared->page_status[bestslot] ==
SLRU_PAGE_CLEAN)            return bestslot;
 
while bestslot could still be -1.

I see your concern about multiple processes selecting the same buffer
for replacement, but what will actually happen is that all but the first
will block for the first one's I/O to complete using SimpleLruWaitIO,
and then all of them will repeat the outer loop and recheck what to do.
If they were all trying to swap in the same page this is actually
optimal.  If they were trying to swap in different pages then the losing
processes will again try to initiate I/O on a different buffer.  (They
will pick a different buffer, because the guy who got the buffer will
have done SlruRecentlyUsed on it before releasing the control lock ---
so I don't believe the worry that we get a buffer thrash scenario here.
Look at the callers of SlruSelectLRUPage not just the function itself.)

It's possible that letting different processes initiate I/O on different
buffers would be a win, but it might just result in excess writes,
depending on the relative probability of requests for the same page
vs. requests for different pages.

Also, I think the patch as posted would still cause processes to gang up
on the same buffer, it would just be a different one from before.  The
right thing would be to locate the overall-oldest buffer and return it
if clean; otherwise to initiate I/O on the oldest buffer that isn't
either clean or write-busy, if there is one; otherwise just do WaitIO
on the oldest buffer.  This would ensure that different processes try
to push different buffers to disk.  They'd still go back and make their
decisions from the top after doing their I/O.  Whether this is a win or
not is not clear to me, but at least it would attack the guessed-at
problem correctly.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: pg_terminate_backend
Next
From: Joe Conway
Date:
Subject: Re: [BUGS] Patch to allow C extension modules to initialize/finish