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.
With Regards,
Amit Kapila.
