On Wed, Nov 14, 2012 at 06:28:41PM -0500, Bruce Momjian wrote: > On Wed, Nov 14, 2012 at 06:15:30PM -0500, Tom Lane wrote: > > Bruce Momjian <br...@momjian.us> writes: > > > On Wed, Nov 14, 2012 at 05:39:29PM -0500, Tom Lane wrote: > > >> You would have been better off keeping the array and sorting it so you > > >> could use binary search, instead of passing the problem off to the > > >> filesystem. > > > > > Well, testing showed using open() was a big win. > > > > ... on the filesystem you tested on. I'm concerned that it might not > > look so good on other platforms. > > True. I am on ext3. So I need to generate a proof-of-concept patch and > have others test it?
OK, test patch attached. The patch will only work if your database uses
a single tablespace. Doing multiple tablespaces seemed too complex for
the test patch.
Here are my results:
# tables git bsearch patch
1 11.16 10.99
1000 19.13 19.26
2000 26.78 27.11
4000 43.81 42.15
8000 79.96 77.38
16000 165.26 162.24
32000 378.18 368.49
64000 1083.35 1086.77
As you can see, the bsearch code doesn't see to make much difference.
Sometimes it is faster, other times, slower --- seem to be just
measurement noise. This code uses the same method of file lookup as git
head, meaning it looks for specific files rather than patterns --- this
simplified the bsearch code.
Can anyone see a consistent improvement with the bsearch patch?
Attached is my test shell script. There is no reason we can't use
bsearch(), except not using it allows us to remove the pg_upgrade
directory listing function.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/file.c b/contrib/pg_upgrade/file.c
new file mode 100644
index d8cd8f5..a5d92c6
*** a/contrib/pg_upgrade/file.c
--- b/contrib/pg_upgrade/file.c
*************** copy_file(const char *srcfile, const cha
*** 221,226 ****
--- 221,281 ----
#endif
+ /*
+ * load_directory()
+ *
+ * Read all the file names in the specified directory, and return them as
+ * an array of "char *" pointers. The array address is returned in
+ * *namelist, and the function result is the count of file names.
+ *
+ * To free the result data, free each (char *) array member, then free the
+ * namelist array itself.
+ */
+ int
+ load_directory(const char *dirname, char ***namelist)
+ {
+ DIR *dirdesc;
+ struct dirent *direntry;
+ int count = 0;
+ int allocsize = 64; /* initial array size */
+
+ *namelist = (char **) pg_malloc(allocsize * sizeof(char *));
+
+ if ((dirdesc = opendir(dirname)) == NULL)
+ pg_log(PG_FATAL, "could not open directory \"%s\": %s\n",
+ dirname, getErrorText(errno));
+
+ while (errno = 0, (direntry = readdir(dirdesc)) != NULL)
+ {
+ if (count >= allocsize)
+ {
+ allocsize *= 2;
+ *namelist = (char **)
+ pg_realloc(*namelist, allocsize * sizeof(char *));
+ }
+
+ (*namelist)[count++] = pg_strdup(direntry->d_name);
+ }
+
+ #ifdef WIN32
+ /*
+ * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
+ * released version
+ */
+ if (GetLastError() == ERROR_NO_MORE_FILES)
+ errno = 0;
+ #endif
+
+ if (errno)
+ pg_log(PG_FATAL, "could not read directory \"%s\": %s\n",
+ dirname, getErrorText(errno));
+
+ closedir(dirdesc);
+
+ return count;
+ }
+
+
void
check_hard_link(void)
{
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index ace56e5..0248d40
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
***************
*** 7,12 ****
--- 7,13 ----
#include <unistd.h>
#include <assert.h>
+ #include <dirent.h>
#include <sys/stat.h>
#include <sys/time.h>
*************** const char *setupPageConverter(pageCnvCt
*** 365,370 ****
--- 366,372 ----
typedef void *pageCnvCtx;
#endif
+ int load_directory(const char *dirname, char ***namelist);
const char *copyAndUpdateFile(pageCnvCtx *pageConverter, const char *src,
const char *dst, bool force);
const char *linkAndUpdateFile(pageCnvCtx *pageConverter, const char *src,
diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c
new file mode 100644
index 7dbaac9..b5b7380
*** a/contrib/pg_upgrade/relfilenode.c
--- b/contrib/pg_upgrade/relfilenode.c
***************
*** 18,24 ****
static void transfer_single_new_db(pageCnvCtx *pageConverter,
FileNameMap *maps, int size);
static void transfer_relfile(pageCnvCtx *pageConverter, FileNameMap *map,
! const char *suffix);
/*
--- 18,24 ----
static void transfer_single_new_db(pageCnvCtx *pageConverter,
FileNameMap *maps, int size);
static void transfer_relfile(pageCnvCtx *pageConverter, FileNameMap *map,
! const char *suffix, char **namelist, int numFiles);
/*
*************** get_pg_database_relfilenode(ClusterInfo
*** 120,125 ****
--- 120,131 ----
PQfinish(conn);
}
+ int scmp(const void *sp1, const void *sp2);
+ int
+ scmp(const void *sp1, const void *sp2)
+ {
+ return( strcmp(*(char **)sp1, *(char **)sp2) );
+ }
/*
* transfer_single_new_db()
*************** static void
*** 130,150 ****
transfer_single_new_db(pageCnvCtx *pageConverter,
FileNameMap *maps, int size)
{
int mapnum;
! bool vm_crashsafe_match = true;
!
! /*
! * Do the old and new cluster disagree on the crash-safetiness of the vm
! * files? If so, do not copy them.
! */
if (old_cluster.controldata.cat_ver < VISIBILITY_MAP_CRASHSAFE_CAT_VER &&
new_cluster.controldata.cat_ver >= VISIBILITY_MAP_CRASHSAFE_CAT_VER)
! vm_crashsafe_match = false;
for (mapnum = 0; mapnum < size; mapnum++)
{
/* transfer primary file */
! transfer_relfile(pageConverter, &maps[mapnum], "");
/* fsm/vm files added in PG 8.4 */
if (GET_MAJOR_VERSION(old_cluster.major_version) >= 804)
--- 136,162 ----
transfer_single_new_db(pageCnvCtx *pageConverter,
FileNameMap *maps, int size)
{
+ char **namelist = NULL;
+ int numFiles = 0;
int mapnum;
! bool vm_crashsafe_change = false;
! int fileno;
!
! /* Do not copy non-crashsafe vm files for binaries that assume crashsafety */
if (old_cluster.controldata.cat_ver < VISIBILITY_MAP_CRASHSAFE_CAT_VER &&
new_cluster.controldata.cat_ver >= VISIBILITY_MAP_CRASHSAFE_CAT_VER)
! vm_crashsafe_change = true;
!
! if (size > 0)
! {
! numFiles = load_directory(maps[0].old_dir, &namelist);
! qsort(namelist, numFiles, sizeof(char *), &scmp);
! }
for (mapnum = 0; mapnum < size; mapnum++)
{
/* transfer primary file */
! transfer_relfile(pageConverter, &maps[mapnum], "", namelist, numFiles);
/* fsm/vm files added in PG 8.4 */
if (GET_MAJOR_VERSION(old_cluster.major_version) >= 804)
*************** transfer_single_new_db(pageCnvCtx *pageC
*** 152,162 ****
/*
* Copy/link any fsm and vm files, if they exist
*/
! transfer_relfile(pageConverter, &maps[mapnum], "_fsm");
! if (vm_crashsafe_match)
! transfer_relfile(pageConverter, &maps[mapnum], "_vm");
}
}
}
--- 164,181 ----
/*
* Copy/link any fsm and vm files, if they exist
*/
! transfer_relfile(pageConverter, &maps[mapnum], "_fsm", namelist, numFiles);
! if (!vm_crashsafe_change)
! transfer_relfile(pageConverter, &maps[mapnum], "_vm", namelist, numFiles);
}
}
+
+ if (numFiles > 0)
+ {
+ for (fileno = 0; fileno < numFiles; fileno++)
+ pg_free(namelist[fileno]);
+ pg_free(namelist);
+ }
}
*************** transfer_single_new_db(pageCnvCtx *pageC
*** 167,178 ****
*/
static void
transfer_relfile(pageCnvCtx *pageConverter, FileNameMap *map,
! const char *type_suffix)
{
const char *msg;
char old_file[MAXPGPATH];
char new_file[MAXPGPATH];
! int fd;
int segno;
char extent_suffix[65];
--- 186,197 ----
*/
static void
transfer_relfile(pageCnvCtx *pageConverter, FileNameMap *map,
! const char *type_suffix, char **namelist, int numFiles)
{
const char *msg;
char old_file[MAXPGPATH];
char new_file[MAXPGPATH];
! char *old_file_p = old_file;
int segno;
char extent_suffix[65];
*************** transfer_relfile(pageCnvCtx *pageConvert
*** 190,217 ****
else
snprintf(extent_suffix, sizeof(extent_suffix), ".%d", segno);
snprintf(old_file, sizeof(old_file), "%s/%u%s%s", map->old_dir,
map->old_relfilenode, type_suffix, extent_suffix);
snprintf(new_file, sizeof(new_file), "%s/%u%s%s", map->new_dir,
map->new_relfilenode, type_suffix, extent_suffix);
- /* Is it an extent, fsm, or vm file? */
- if (type_suffix[0] != '\0' || segno != 0)
- {
- /* Did file open fail? */
- if ((fd = open(old_file, O_RDONLY, 0)) == -1)
- {
- /* File does not exist? That's OK, just return */
- if (errno == ENOENT)
- return;
- else
- pg_log(PG_FATAL, "error while checking for file existance \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
- map->nspname, map->relname, old_file, new_file,
- getErrorText(errno));
- }
- close(fd);
- }
-
unlink(new_file);
/* Copying files might take some time, so give feedback. */
--- 209,229 ----
else
snprintf(extent_suffix, sizeof(extent_suffix), ".%d", segno);
+ /* Is it an extent, fsm, or vm file? return if it does not exist */
+ if (type_suffix[0] != '\0' || segno != 0)
+ {
+ /* create file name without path to compare */
+ snprintf(old_file, sizeof(old_file), "%u%s%s",
+ map->old_relfilenode, type_suffix, extent_suffix);
+ if (bsearch(&old_file_p, namelist, numFiles, sizeof(char *), &scmp) == NULL)
+ return;
+ }
+
snprintf(old_file, sizeof(old_file), "%s/%u%s%s", map->old_dir,
map->old_relfilenode, type_suffix, extent_suffix);
snprintf(new_file, sizeof(new_file), "%s/%u%s%s", map->new_dir,
map->new_relfilenode, type_suffix, extent_suffix);
unlink(new_file);
/* Copying files might take some time, so give feedback. */
test.sh
Description: Bourne shell script
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
