Re: WIP patch for hint bit i/o mitigation - Mailing list pgsql-hackers

From Merlin Moncure
Subject Re: WIP patch for hint bit i/o mitigation
Date
Msg-id CAHyXU0y=Mvm2XzdLcjy=4=Hfw1O1UN2Z4qdWYFaEM2zk7ZOO+A@mail.gmail.com
Whole thread Raw
In response to WIP patch for hint bit i/o mitigation  (Merlin Moncure <mmoncure@gmail.com>)
Responses Re: WIP patch for hint bit i/o mitigation  (Atri Sharma <atri.jiit@gmail.com>)
Re: WIP patch for hint bit i/o mitigation  (Greg Smith <greg@2ndQuadrant.com>)
List pgsql-hackers
On Fri, Nov 16, 2012 at 4:32 AM, Amit Kapila <amit.kapila@huawei.com> wrote:
> On Thursday, November 15, 2012 10:19 PM Merlin Moncure wrote:
>> On Thu, Nov 15, 2012 at 10:25 AM, Amit Kapila <amit.kapila@huawei.com>
>> wrote:
>
>>
>> Sure, although in scans we are using ring buffer as well so in
>> practical sense the results are pretty close.
>>
>> > b. Considering sometimes people want VACUUM to run when system is not
>> busy,
>> > the chances of generating more overall I/O in system can be
>> >    more.
>>
>> It's hard to imagine overall i/o load increasing.  However, longer
>> vacuum times should be considered.   A bigger  issue is that we are
>> missing an early opportunity to set the all visible bit. vacuum is
>> doing:
>>
>> if (all_visible)
>>   {
>>           TransactionId xmin;
>>
>>           if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
>>           {
>>                   all_visible = false;
>>                   break;
>>           }
>>
>> assuming the cache is working and vacuum rolls around after a scan,
>> you lost the opportunity to set all_visible flag where previously it
>> would have been set, thereby dismantling the positive effect of an
>> index only scan.  AFAICT, this is the only case where vaccum is
>> directly interfacing with hint bits.  This could be construed as a
>> violation of heapam API?  Maybe if that's an issue we could proxy that
>> check to a heaptuple/tqual.c maintained function (in the same manner
>> as HeapTupleSetHintBits) so that the cache bit could be uniformly
>> checked.
>
> I think we need to think of some tests to check if Vacuum or any other
> impact has not been created due to this change.
> I will devise tests during review of this patch.
> However if you have more ideas then share the same which will make tests of
> this patch more strong.
> For functional/performance test of this patch, one of my colleague Hari Babu
> will also work along with me on it.

Thanks.  So far, Atri ran some quick n dirty tests to see if there
were any regressions.  He benched a large scan followed by vacuum.  So
far, results are inconclusive so better testing methodologies will
definitely be greatly appreciated.  One of the challenges with working
in this part of the code is that it's quite difficult to make changes
without impacting at least some workloads.

merlin



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Switching timeline over streaming replication
Next
From: Andres Freund
Date:
Subject: Re: logical changeset generation v3 - comparison to Postgres-R change set format