Re: [WIP PATCH] for Performance Improvement in Buffer Management

From: Amit kapila <amit(dot)kapila(at)huawei(dot)com>
To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [WIP PATCH] for Performance Improvement in Buffer Management
Date: 2012-09-04 13:25:34
Message-ID: 6C0B27F7206C9E4CA54AE035729E9C3828530351@szxeml509-mbs
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tuesday, September 04, 2012 12:42 AM Jeff Janes wrote:
On Mon, Sep 3, 2012 at 7:15 AM, Amit kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
>> This patch is based on below Todo Item:
>
>> Consider adding buffers the background writer finds reusable to the free
>> list
>
>
>
>> I have tried implementing it and taken the readings for Select when all the
>> data is in either OS buffers
>
>> or Shared Buffers.
>
>
>
>> The Patch has simple implementation for "bgwriter or checkpoint process
>> moving the unused buffers (unpinned with "ZERO" usage_count buffers) into
>> "freelist".

> I don't think InvalidateBuffer can be safely used in this way. It
> says "We assume
> that no other backend could possibly be interested in using the page",
> which is not true here.

As I understood and anlyzed based on above, that there is problem in attached patch such that in function
InvalidateBuffer(), after UnlockBufHdr() and before PartitionLock if some backend uses that buffer and increase the usage count to 1, still
InvalidateBuffer() will remove the buffer from hash table and put it in Freelist.
I have modified the code to address above by checking refcount & usage_count inside Partition Lock
, LockBufHdr and only after that move it to freelist which is similar to InvalidateBuffer.
In actual code we can optimize the current code by using extra parameter in InvalidateBuffer.

Please let me know if I understood you correctly or you want to say something else by above comment?

> Also, do we want to actually invalidate the buffers? If someone does
> happen to want one after it is put on the freelist, making it read it
> in again into a different buffer doesn't seem like a nice thing to do,
> rather than just letting it reclaim it.

But even if bgwriter/checkpoint don't do, Backend needing new buffer will do similar things (remove from hash table) for this buffer as this is nextvictim buffer.
The main intention of doing the MoveBufferToFreeList is to avoid contention of Partition Locks and BufFreeListLock among backends, which
has given Performance improvement in high contention scenarios.

One problem I could see with proposed change is that in some cases the usage count will get decrement for a buffer allocated
from free list immediately as it can be nextvictimbuffer.
However there can be solution to this problem.

Can you suggest some scenario's where I should do more performance test?

With Regards,
Amit Kapila.

Attachment Content-Type Size
bgwritter_or_checkpoint_proc_moving_buf_to_freelist.2.patch text/plain 3.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2012-09-04 13:27:00 Re: Statistics and selectivity estimation for ranges
Previous Message Bruce Momjian 2012-09-04 13:21:13 pg_upgrade docs