On Thu, Dec 13, 2012 at 7:06 AM, Hari Babu <haribabu.ko...@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' -- I'd like to see the bulk insert performance hit reduced if possible. Let's see what we can do in the short term (and, if no improvement can be found, I think this patch should be marked 'returned with feedback'). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
