From: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
---|---|
To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Merlin Moncure <mmoncure(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, YUriy Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Move PinBuffer and UnpinBuffer to atomics |
Date: | 2016-03-25 14:39:46 |
Message-ID: | CAPpHfdsTFD8XvGica8iKxgaiMTZco46nkhofeP7QkW7seaC-6A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Mar 22, 2016 at 1:08 PM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Tue, Mar 22, 2016 at 7:57 AM, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
> wrote:
>
>>
>> On Tue, Mar 22, 2016 at 12:31 PM, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
>> wrote:
>>
>>> ! pg_atomic_write_u32(&bufHdr->state, state);
>>> } while (!StartBufferIO(bufHdr, true));
>>>
>>> Better Write some comment, about we clearing the BM_LOCKED from stage
>>> directly and need not to call UnlockBufHdr explicitly.
>>> otherwise its confusing.
>>>
>>
>> Few more comments..
>>
>> *** 828,837 ****
>> */
>> do
>> {
>> ! LockBufHdr(bufHdr);
>> *! Assert(bufHdr->flags & BM_VALID);*
>> ! bufHdr->flags &= ~BM_VALID;
>> ! UnlockBufHdr(bufHdr);
>> } while (!StartBufferIO(bufHdr, true));
>> }
>> }
>> --- 826,834 ----
>> */
>> do
>> {
>> ! uint32 state = LockBufHdr(bufHdr);
>> ! state &= ~(BM_VALID | BM_LOCKED);
>> ! pg_atomic_write_u32(&bufHdr->state, state);
>> } while (!StartBufferIO(bufHdr, true));
>>
>> 1. Previously there was a Assert, any reason why we removed it ?
>> Assert(bufHdr->flags & BM_VALID);
>>
>
> It was missed. In the attached patch I've put it back.
>
> 2. And also if we don't need assert then can't we directly clear BM_VALID
>> flag from state variable (if not locked) like pin/unpin buffer does,
>> without taking buffer header lock?
>>
>
> In this version of patch it could be also done as loop of CAS operation.
> But I'm not intended to it so because it would significantly complicate
> code. It's not yes clear that traffic in this place is high enough to make
> such optimizations.
> Since v4 patch implements slightly different approach. Could you please
> test it? We need to check that this approach worth putting more efforts on
> it. Or through it away otherwise.
>
Could anybody run benchmarks? Feature freeze is soon, but it would be
*very nice* to fit it into 9.6 release cycle, because it greatly improves
scalability on large machines. Without this patch PostgreSQL 9.6 will be
significantly behind competitors like MySQL 5.7.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From | Date | Subject | |
---|---|---|---|
Next Message | Onder Kalaci | 2016-03-25 14:49:02 | A question on systable_beginscan() |
Previous Message | Alexander Korotkov | 2016-03-25 14:32:14 | Re: WIP: Access method extendability |