From: Greg Stark <gsstark@mit.edu>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, pgsql-performance@postgresql.org, Michael Clemmons <glassresistor@gmail.com>
Cc: Hannu Krosing <hannu@2ndquadrant.com>
Cc: Scott Marlowe <scott.marlowe@gmail.com>
Subject: Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
Date: Tue, 19 Jan 2010 14:52:25 +0000
Message-ID: <407d949e1001190652i43f4f276x6a485f375647219a@mail.gmail.com> (raw)
In-Reply-To: <407d949e1001180835m79ffe3e4w53e20ce2e1a58f2d@mail.gmail.com>
References: <4427a97a0912101235x3ade510blcc00d5ce9883c6b6@mail.gmail.com>
<200912282354.51892.andres@anarazel.de>
<200912282359.43979.andres@anarazel.de>
<200912291930.50812.andres@anarazel.de>
<407d949e1001180835m79ffe3e4w53e20ce2e1a58f2d@mail.gmail.com>
On Mon, Jan 18, 2010 at 4:35 PM, Greg Stark <gsstark@mit.edu> wrote:
> Looking at this patch for the commitfest I have a few questions.
So I've touched this patch up a bit:
1) moved the posix_fadvise call to a new fd.c function
pg_fsync_start(fd,offset,nbytes) which initiates an fsync without
waiting on it. Currently it's only implemented with
posix_fadvise(DONT_NEED) but I want to look into using sync_file_range
in the future -- it looks like this call might be good enough for our
checkpoints.
2) advised each 64k chunk as we write it which should avoid poisoning
the cache if you do a large create database on an active system.
3) added the promised but afaict missing fsync of the directory -- i
think we should actually backpatch this.
Barring any objections shall I commit it like this?
--
greg
--
greg
Attachments:create-database-speedup-using-pfa.diff (text/x-patch, 4.7K) download
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
***************
*** 319,324 **** pg_fdatasync(int fd)
--- 319,340 ----
return 0;
}
+ int+ pg_fsync_start(int fd, off_t offset, off_t amount)+ {+ if (enableFsync)+ {+ #if defined(USE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)+ return posix_fadvise(fd, offset, amount, POSIX_FADV_DONTNEED);+ #else+ return 0;+ #endif+ }+ else+ return 0;+ }+ +
/*
* InitFileAccess --- initialize this module during backend startup
*
*** a/src/include/storage/fd.h
--- b/src/include/storage/fd.h
***************
*** 98,103 **** extern int pg_fsync(int fd);
--- 98,104 ----
extern int pg_fsync_no_writethrough(int fd);
extern int pg_fsync_writethrough(int fd);
extern int pg_fdatasync(int fd);
+ extern int pg_fsync_start(int fd, off_t offset, off_t amount);
/* Filename components for OpenTemporaryFile */
#define PG_TEMP_FILES_DIR "pgsql_tmp"
*** a/src/port/copydir.c
--- b/src/port/copydir.c
***************
*** 37,42 ****
--- 37,43 ----
static void copy_file(char *fromfile, char *tofile);
+ static void fsync_fname(char *fname);
/*
***************
*** 64,69 **** copydir(char *fromdir, char *todir, bool recurse)
--- 65,73 ----
(errcode_for_file_access(),
errmsg("could not open directory \"%s\": %m", fromdir)));
+ /*+ * Copy all the files+ */
while ((xlde = ReadDir(xldir, fromdir)) != NULL)
{
struct stat fst;
***************
*** 89,96 **** copydir(char *fromdir, char *todir, bool recurse)
--- 93,127 ----
else if (S_ISREG(fst.st_mode))
copy_file(fromfile, tofile);
}
+ FreeDir(xldir);+ + /*+ * Be paranoid here and fsync all files to ensure we catch problems.+ */+ xldir = AllocateDir(fromdir);+ if (xldir == NULL)+ ereport(ERROR,+ (errcode_for_file_access(),+ errmsg("could not open directory \"%s\": %m", fromdir)));+ + while ((xlde = ReadDir(xldir, fromdir)) != NULL)+ {+ if (strcmp(xlde->d_name, ".") == 0 ||+ strcmp(xlde->d_name, "..") == 0)+ continue;+ snprintf(tofile, MAXPGPATH, "%s/%s", todir, xlde->d_name);+ fsync_fname(tofile);+ }
FreeDir(xldir);
+ + /* It's important to fsync the destination directory itself as+ * individual file fsyncs don't guarantee that the directory entry+ * for the file is synced. Recent versions of ext4 have made the+ * window much wider but it's been true for ext3 and other+ * filesyetems in the past + */+ fsync_fname(todir);
}
/*
***************
*** 103,108 **** copy_file(char *fromfile, char *tofile)
--- 134,140 ----
int srcfd;
int dstfd;
int nbytes;
+ off_t offset;
/* Use palloc to ensure we get a maxaligned buffer */
#define COPY_BUF_SIZE (8 * BLCKSZ)
***************
*** 128,134 **** copy_file(char *fromfile, char *tofile)
/*
* Do the data copying.
*/
! for (;;)
{
nbytes = read(srcfd, buffer, COPY_BUF_SIZE);
if (nbytes < 0)
--- 160,166 ----
/*
* Do the data copying.
*/
! for (offset=0; ; offset+=nbytes)
{
nbytes = read(srcfd, buffer, COPY_BUF_SIZE);
if (nbytes < 0)
***************
*** 147,161 **** copy_file(char *fromfile, char *tofile)
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tofile)));
}
- }
! /*
! * Be paranoid here to ensure we catch problems.
! */
! if (pg_fsync(dstfd) != 0)
! ereport(ERROR,
! (errcode_for_file_access(),
! errmsg("could not fsync file \"%s\": %m", tofile)));
if (close(dstfd))
ereport(ERROR,
--- 179,195 ----
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tofile)));
}
! /*
! * Avoid poisoning the cache with the entire database just
! * like we do for large sequential scans. Actually that's a
! * white lie, the real motivation for this is that it might
! * urge the kernel to flush these buffers when convenient
! * before we get to the later fsyncs. Empirical results seem
! * to show a big speed benefit on recent Linux kernels.
! */
! pg_fsync_start(dstfd, offset, nbytes);
! }
if (close(dstfd))
ereport(ERROR,
***************
*** 166,168 **** copy_file(char *fromfile, char *tofile)
--- 200,224 ----
pfree(buffer);
}
+ + /*+ * fsync a file+ */+ static void+ fsync_fname(char *fname)+ {+ int fd = BasicOpenFile(fname, + O_RDONLY | PG_BINARY,+ S_IRUSR | S_IWUSR);+ + if (fd < 0)+ ereport(ERROR,+ (errcode_for_file_access(),+ errmsg("could not open file \"%s\": %m", fname)));+ + if (pg_fsync(fd) != 0)+ ereport(ERROR,+ (errcode_for_file_access(),+ errmsg("could not fsync file \"%s\": %m", fname)));+ close(fd);+ }
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: gsstark@mit.edu, andres@anarazel.de, glassresistor@gmail.com, hannu@2ndquadrant.com, scott.marlowe@gmail.com
Subject: Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
In-Reply-To: <407d949e1001190652i43f4f276x6a485f375647219a@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox