Skip site navigation (1) Skip section navigation (2)

Re: WIP patch for hint bit i/o mitigation

From: Atri Sharma <atri(dot)jiit(at)gmail(dot)com>
To: Merlin Moncure <mmoncure(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP patch for hint bit i/o mitigation
Date: 2012-11-16 19:42:26
Message-ID: CAOeZVifze2OxHfG+_JaSTQ4F=4uND5CopKknrns=Qi3U4iCBwg@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Fri, Nov 16, 2012 at 7:33 PM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:

> On Fri, Nov 16, 2012 at 4:32 AM, Amit Kapila <amit(dot)kapila(at)huawei(dot)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(dot)kapila(at)huawei(dot)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
>

Thanks a ton Amit and your colleague Hari for volunteering to review the
patch.

I ran some benchmarks and came up with the following results:

With our code

atri(at)atri-Aspire-4740:~/postgresql-9.2.1/contrib/pgbench$ ./pgbench atri1
-n -f bench2.sql -c 8 -T 300
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 8
number of threads: 1
duration: 300 s
number of transactions actually processed: 412
tps = 1.366142 (including connections establishing)
tps = 1.366227 (excluding connections establishing)


Without our code

atri(at)atri-Aspire-4740:~/postgresql-9.2.1/contrib/pgbench$ ./pgbench atri1
-n -f bench2.sql -c 8 -T 300
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 8
number of threads: 1
duration: 300 s
number of transactions actually processed: 378
tps = 1.244333 (including connections establishing)
tps = 1.244447 (excluding connections establishing)

The SQL file is attached.

Please let us know if you need any more details.

Atri
-- 
Regards,

Atri
*l'apprenant*

Attachment: bench2.sql
Description: application/octet-stream (123 bytes)

In response to

pgsql-hackers by date

Next:From: Kohei KaiGaiDate: 2012-11-16 20:09:05
Subject: Re: [v9.3] writable foreign tables
Previous:From: Tom LaneDate: 2012-11-16 19:16:24
Subject: Re: another idea for changing global configuration settings from SQL

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group