On Thursday, December 13, 2012 8:02 PM Merlin Moncure wrote:
> On Thu, Dec 13, 2012 at 7:06 AM, Hari Babu <haribabu.kommi@huawei.com>
> wrote:
> > Please find the review of the patch.
>
>
> Thanks for detailed review!
>
> > Basic stuff:
> > ------------
> > - Patch applies with offsets.
> > - Compiles cleanly with no warnings
> > - Regression Test pass.
> >
> > Code Review:
> > -------------
> > 1. Better to set the hint bits for the tuples in a page, if
> the page
> > is already dirty.
>
> This is true today but likely less true if/when page checksums come
> out. Also it complicates the code a little bit.
>
> > Default tables select : 64980.736149 64550.118693
> > Unlogged tables select: 64874.974334 64550.118693
>
> So it looks like the extra tests visibility routines are causing 0.7%
> performance hit.
>
> > Multiple transaction bulk inserts: Select performance (refer script -1
> & 2
> > which attached)
> > sequential scan: 6.492680 6.382014
> > Index scan: 1.386851 1.36234
> >
> > Single transaction bulk inserts: Select performance (refer script - 3
> & 4
> > which attached)
> > sequential scan: 6.49319 6.3800147
> > Index scan: 1.384121 1.3615277
>
> The performance hit is higher here. Almost 2%. This is troubling.
>
> > Long transaction open then Vacuum & select performance in milli
> seconds.
> > (refer reports output)
> > Testcase - 3:
> > Single Vacuum Perf : 128.302 ms 181.292 ms
> > Single select perf : 214.107 ms 177.057 ms
> > Total : 342.409 ms 358.349 ms
> >
> > I was not able to find the reason why in some of cases results are low
> so
> > please use the attached scripts to validate the same.
>
> I need to validate the vacuum results. It's possible that this is
> solvable by tweaking xmin check inside vacuum. Assuming that's fixed,
> the question stands: do the results justify the change? I'd argue
> 'maybe'
We can try with change (assuming change is small) and see if the performance
gain is good, then discuss whether it really justifies.
I think the main reason for Vacuum performance hit is that in the test pages
are getting dirty only due to setting of hint bit
by Vacuum.
>-- I'd like to see the bulk insert performance hit reduced if
> possible.
I think if we can improve performance for bulk-insert case, then this patch
has much more value.
With Regards,
Amit Kapila.