Thread: HOT Patch - Ready for review

HOT Patch - Ready for review

From
"Pavan Deolasee"
Date:

Please find the attached HOT patch, which I think is now ready for
review.

The only known issue left to-be-done  is making the index available
in the read-committed transaction which created it. Right now the index
is made available to the transaction unless  one or more rows were
UPDATEd in the same transactions before creating the index OR there were
RECENTLY_DEAD tuples that we did not index while building the index.
Both of these cases do not seem very common to me.The patch can easily
be tweaked to make the index available even in these cases, but I left it
because Tom has raised concerns about transaction using the new index
with some old snapshot. I haven't been able to create such a scenario
so thought would best leave it until someone picks it up for review and
take it up at that time.

One of the regression test fails, but thats just a side-effect of how CIC
works now. I didn't change the expected output just yet because that might
have covered this side-effect. I plan to spend some time on performance
testing and would post those results soon.

I would appreciate if others also give it a shot and see if it gives any
performance jump in their respective setups. I think the best would
come out in IO bound testing.

Thanks,
Pavan

--

EnterpriseDB     http://www.enterprisedb.com
Attachment

Re: HOT Patch - Ready for review

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Pavan Deolasee wrote:
> Please find the attached HOT patch, which I think is now ready for
> review.
>
> The only known issue left to-be-done  is making the index available
> in the read-committed transaction which created it. Right now the index
> is made available to the transaction unless  one or more rows were
> UPDATEd in the same transactions before creating the index OR there were
> RECENTLY_DEAD tuples that we did not index while building the index.
> Both of these cases do not seem very common to me.The patch can easily
> be tweaked to make the index available even in these cases, but I left it
> because Tom has raised concerns about transaction using the new index
> with some old snapshot. I haven't been able to create such a scenario
> so thought would best leave it until someone picks it up for review and
> take it up at that time.
>
> One of the regression test fails, but thats just a side-effect of how CIC
> works now. I didn't change the expected output just yet because that might
> have covered this side-effect. I plan to spend some time on performance
> testing and would post those results soon.
>
> I would appreciate if others also give it a shot and see if it gives any
> performance jump in their respective setups. I think the best would
> come out in IO bound testing.
>
> Thanks,
> Pavan
>
> --
>
> EnterpriseDB     http://www.enterprisedb.com

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
>        subscribe-nomail command to majordomo@postgresql.org so that your
>        message can get through to the mailing list cleanly

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: HOT Patch - Ready for review

From
Heikki Linnakangas
Date:
Pavan Deolasee wrote:
> Please find the attached HOT patch, which I think is now ready for
> review.

What's the purpose of the "HeapScanHintPagePrune" mechanism in index
builds? I lost track of the discussion on create index, is the it
necessary for correctness? A comment in IndexBuildHeapScan explaining
why it's done would be nice. In any case a PG_TRY/CATCH block should be
used to make sure it's turned off after an unsuccessful index build.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Re: HOT Patch - Ready for review

From
"Pavan Deolasee"
Date:

On 4/19/07, Heikki Linnakangas <heikki@enterprisedb.com> wrote:

What's the purpose of the "HeapScanHintPagePrune" mechanism in index
builds? I lost track of the discussion on create index, is the it
necessary for correctness?

Its not required strictly for correctness, but it helps us prune the HOT-chains
while index building. During index build, if we skip a tuple which is
RECENTLY_DEAD, existing transactions can not use the index for queries.
Pruning the HOT-chains reduces the possibility of finding such tuples
while building the index.
 

A comment in IndexBuildHeapScan explaining
why it's done would be nice.

I would do that.
 

In any case a PG_TRY/CATCH block should be
used to make sure it's turned off after an unsuccessful index build.

Oh thanks. Would do that too
 
I would wait for other review comments before submitting a fresh patch.
I hope thats ok.

Thanks,
Pavan
--

EnterpriseDB     http://www.enterprisedb.com