Re: WIP patch for hint bit i/o mitigation

On Thu, Nov 15, 2012 at 10:25 AM, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
>> IMNSHO. deferring non-critical i/o from foreground process to
>> background process is generally good.
>
> Yes, in regard of deferring you are right.
> However in this case may be when foreground process has to mark page dirty
> due to hint-bit, it was already dirty so no extra I/O, but when it is done
> by VACUUM, page may not be dirty.

Yeah. We can try to be smart and set the hint bits in that case. Not
sure that will work out with checksum having to wal log hint bits
though (which by reading the checksum threads seems likely to happen).

> Also due to below points, doing it in VACUUM may cost more:
> a. VACUUM has ring-buffer of fixed size and if such pages are many then
> write of page needs to be done by VACUUM to replace existing page
> in ring.

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.

All other *setting* of hint bits is running through SetHintBits(), so
I think we are safe from vacuum point of view. That's another thing
to test for though.

merlin