On Monday, June 18, 2012 08:39:47 PM Jeff Davis wrote: > On Mon, 2012-06-18 at 18:05 +0200, Andres Freund wrote: > > Quick review: > > - defaulting to initdb -N in the regression suite is not a good imo, > > because that way the buildfarm won't catch problems in that area... > I removed the -N as you suggest. How much does performance matter on the > buildfarm? I don't think the difference in initdb cost is relevant when running the regression tests. Should it prove to be we can re-add -N after a week or two in the buildfarm machines. I just remember that there were several OS specific regression when adding the pre-syncing for createdb.
> > - could the copydir.c and initdb.c versions of walkdir/sync_fname et al > > be unified? > There's a lot of backend-specific code in the copydir versions, like > using ereport() and CHECK_FOR_INTERRUPTS(). I gave a brief attempt at > unifying them before, and concluded that it wouldn't add to the > readability, so I just commented where they came from. Ok. Sensible reasons. I dislike that we know have two files using different logic (copydir.c only using fadvise, initdb using sync_file_range if available). Maybe we should just move the fadvise and sync_file_range calls into its own common function? > If you feel there will be a maintainability problem, I can give it > another shot. Its not too bad yet I guess, so ... > > - I personally would find it way nicer to put USE_PRE_SYNC into > > pre_sync_fname instead of cluttering the main function with it > Done. Looks good to me. Btw, I just want to have said this, although I don't think its particularly relevant as it doesn't affect correctness: Its possible to have a system where sync_file_range is in the system headers but the kernel during runtime doesn't support it. It is relatively new (2.6.17). It would be possible to fallback to posix_fadvise which has been around far longer in that case... Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
