From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | Jeff Davis <pgsql(at)j-davis(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net> |
Subject: | Re: initdb and fsync |
Date: | 2012-06-18 18:57:51 |
Message-ID: | 201206182057.51876.andres@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2012-06-18 19:19:48 | Re: WAL format changes |
Previous Message | Jeff Davis | 2012-06-18 18:52:25 | Re: initdb and fsync |