Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Katsuhiko Okano <okano(dot)katsuhiko(at)oss(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: LWLock statistics collector (was: CSStorm occurred again by postgreSQL8.2)
Date: 2006-08-03 22:37:34
Message-ID: 27074.1154644654@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Katsuhiko Okano <okano(dot)katsuhiko(at)oss(dot)ntt(dot)co(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2006-08-03 22:45:09 Re: [BUGS] Patch to allow C extension modules to initialize/finish
Previous Message Bruce Momjian 2006-08-03 22:15:32 Re: pg_terminate_backend

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-08-04 00:01:07 Re: regression tests for guc SET
Previous Message Tom Lane 2006-08-03 22:00:20 Re: [PATCHES] Forcing current WAL file to be archived