From f07f0443cd3f0e3892a6d8842094d9b07eb81698 Mon Sep 17 00:00:00 2001 From: jodiew Date: Thu, 20 Mar 2025 18:35:13 +0000 Subject: [PATCH 1/3] Add copy_file_range strategy to create database --- src/backend/commands/dbcommands.c | 19 +++++--- src/backend/storage/file/copydir.c | 72 ++++++++++++++++++++++++++++-- src/include/storage/copydir.h | 3 +- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 2505b3084b069..920d81104efb8 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -83,6 +83,7 @@ typedef enum CreateDBStrategy { CREATEDB_WAL_LOG, CREATEDB_FILE_COPY, + CREATEDB_COPY_FILE_RANGE, } CreateDBStrategy; typedef struct @@ -136,7 +137,8 @@ static CreateDBRelInfo *ScanSourceDatabasePgClassTuple(HeapTupleData *tuple, static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid, bool isRedo); static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, - Oid src_tsid, Oid dst_tsid); + Oid src_tsid, Oid dst_tsid, + bool range_copy); static void recovery_create_dbdir(char *path, bool only_tblspc); /* @@ -548,7 +550,7 @@ CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid, bool isRedo) */ static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, - Oid dst_tsid) + Oid dst_tsid, bool range_copy) { TableScanDesc scan; Relation rel; @@ -608,7 +610,7 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, * * We don't need to copy subdirectories */ - copydir(srcpath, dstpath, false); + copydir(srcpath, dstpath, false, range_copy); /* Record the filesystem change in XLOG */ { @@ -1022,6 +1024,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) dbstrategy = CREATEDB_WAL_LOG; else if (pg_strcasecmp(strategy, "file_copy") == 0) dbstrategy = CREATEDB_FILE_COPY; + else if (pg_strcasecmp(strategy, "copy_file_range") == 0) + dbstrategy = CREATEDB_COPY_FILE_RANGE; else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1508,9 +1512,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (dbstrategy == CREATEDB_WAL_LOG) CreateDatabaseUsingWalLog(src_dboid, dboid, src_deftablespace, dst_deftablespace); + else if (dbstrategy == CREATEDB_COPY_FILE_RANGE) + CreateDatabaseUsingFileCopy(src_dboid, dboid, src_deftablespace, + dst_deftablespace, true); else CreateDatabaseUsingFileCopy(src_dboid, dboid, src_deftablespace, - dst_deftablespace); + dst_deftablespace, false); /* * Close pg_database, but keep lock till commit. @@ -2156,7 +2163,7 @@ movedb(const char *dbname, const char *tblspcname) /* * Copy files from the old tablespace to the new one */ - copydir(src_dbpath, dst_dbpath, false); + copydir(src_dbpath, dst_dbpath, false, false); /* * Record the filesystem change in XLOG @@ -3341,7 +3348,7 @@ dbase_redo(XLogReaderState *record) * * We don't need to copy subdirectories */ - copydir(src_path, dst_path, false); + copydir(src_path, dst_path, false, false); pfree(src_path); pfree(dst_path); diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index d4fbe542077f1..fa9e7ae0dff8c 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -26,6 +26,7 @@ #include "pgstat.h" #include "storage/copydir.h" #include "storage/fd.h" +#include "pg_config.h" /* * copydir: copy a directory @@ -34,7 +35,7 @@ * a directory or a regular file is ignored. */ void -copydir(const char *fromdir, const char *todir, bool recurse) +copydir(const char *fromdir, const char *todir, bool recurse, bool range_copy) { DIR *xldir; struct dirent *xlde; @@ -68,10 +69,15 @@ copydir(const char *fromdir, const char *todir, bool recurse) { /* recurse to handle subdirectories */ if (recurse) - copydir(fromfile, tofile, true); + copydir(fromfile, tofile, true, range_copy); } else if (xlde_type == PGFILETYPE_REG) - copy_file(fromfile, tofile); + { + if (range_copy) + copy_file_reflink(fromfile, tofile); + else + copy_file(fromfile, tofile); + } } FreeDir(xldir); @@ -214,3 +220,63 @@ copy_file(const char *fromfile, const char *tofile) pfree(buffer); } + +/* + * copy one file using copy_file_range to give filesystem a chance to do COW optimization + */ +void +copy_file_reflink(const char *fromfile, const char *tofile) +{ +#if defined(HAVE_COPY_FILE_RANGE) + int srcfd; + int dstfd; + ssize_t nbytes; + + /* + * Open the files + */ + srcfd = OpenTransientFile(fromfile, O_RDONLY | PG_BINARY); + + if (srcfd < 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not open file \"%s\": %m", fromfile)); + + dstfd = OpenTransientFile(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY); + + if (dstfd < 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not create file \"%s\": %m", tofile)); + + /* + * Do the data copying. + */ + do + { + /* If we got a cancel signal during the copy of the file, quit */ + CHECK_FOR_INTERRUPTS(); + + pgstat_report_wait_start(WAIT_EVENT_COPY_FILE_READ); + nbytes = copy_file_range(srcfd, NULL, dstfd, NULL, SSIZE_MAX, 0); + pgstat_report_wait_end(); + if (nbytes < 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not copy file \"%s\": %m", fromfile)); + } + while (nbytes > 0); + + if (CloseTransientFile(dstfd) != 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not close file \"%s\": %m", tofile)); + + if (CloseTransientFile(srcfd) != 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not close file \"%s\": %m", fromfile)); +#else + pg_fatal("copy_file_range not available on this platform"); +#endif +} diff --git a/src/include/storage/copydir.h b/src/include/storage/copydir.h index a25e258f4797e..d4d5a0063aeff 100644 --- a/src/include/storage/copydir.h +++ b/src/include/storage/copydir.h @@ -13,7 +13,8 @@ #ifndef COPYDIR_H #define COPYDIR_H -extern void copydir(const char *fromdir, const char *todir, bool recurse); +extern void copydir(const char *fromdir, const char *todir, bool recurse, bool range_copy); extern void copy_file(const char *fromfile, const char *tofile); +extern void copy_file_reflink(const char *fromfile, const char *tofile); #endif /* COPYDIR_H */ From ffc709ae1e8cdc8743464e457c55db9559d4c0d3 Mon Sep 17 00:00:00 2001 From: jodiew Date: Thu, 20 Mar 2025 23:52:18 +0000 Subject: [PATCH 2/3] Improve naming of some file copy functions and parameters --- src/backend/commands/dbcommands.c | 20 ++++++++++---------- src/backend/storage/file/copydir.c | 21 ++++++++++++++++----- src/include/storage/copydir.h | 14 ++++++++++++-- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 920d81104efb8..2013bd6c9aea1 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -83,7 +83,7 @@ typedef enum CreateDBStrategy { CREATEDB_WAL_LOG, CREATEDB_FILE_COPY, - CREATEDB_COPY_FILE_RANGE, + CREATEDB_FILE_COPY_RANGE, } CreateDBStrategy; typedef struct @@ -138,7 +138,7 @@ static void CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid, bool isRedo); static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, Oid dst_tsid, - bool range_copy); + CopyMethod copy_method); static void recovery_create_dbdir(char *path, bool only_tblspc); /* @@ -550,7 +550,7 @@ CreateDirAndVersionFile(char *dbpath, Oid dbid, Oid tsid, bool isRedo) */ static void CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, - Oid dst_tsid, bool range_copy) + Oid dst_tsid, CopyMethod copy_method) { TableScanDesc scan; Relation rel; @@ -610,7 +610,7 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, * * We don't need to copy subdirectories */ - copydir(srcpath, dstpath, false, range_copy); + copydir_with_method(srcpath, dstpath, false, copy_method); /* Record the filesystem change in XLOG */ { @@ -1025,7 +1025,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) else if (pg_strcasecmp(strategy, "file_copy") == 0) dbstrategy = CREATEDB_FILE_COPY; else if (pg_strcasecmp(strategy, "copy_file_range") == 0) - dbstrategy = CREATEDB_COPY_FILE_RANGE; + dbstrategy = CREATEDB_FILE_COPY_RANGE; else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1512,12 +1512,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (dbstrategy == CREATEDB_WAL_LOG) CreateDatabaseUsingWalLog(src_dboid, dboid, src_deftablespace, dst_deftablespace); - else if (dbstrategy == CREATEDB_COPY_FILE_RANGE) + else if (dbstrategy == CREATEDB_FILE_COPY_RANGE) CreateDatabaseUsingFileCopy(src_dboid, dboid, src_deftablespace, - dst_deftablespace, true); + dst_deftablespace, COPY_METHOD_COPY_FILE_RANGE); else CreateDatabaseUsingFileCopy(src_dboid, dboid, src_deftablespace, - dst_deftablespace, false); + dst_deftablespace, COPY_METHOD_COPY); /* * Close pg_database, but keep lock till commit. @@ -2163,7 +2163,7 @@ movedb(const char *dbname, const char *tblspcname) /* * Copy files from the old tablespace to the new one */ - copydir(src_dbpath, dst_dbpath, false, false); + copydir(src_dbpath, dst_dbpath, false); /* * Record the filesystem change in XLOG @@ -3348,7 +3348,7 @@ dbase_redo(XLogReaderState *record) * * We don't need to copy subdirectories */ - copydir(src_path, dst_path, false, false); + copydir(src_path, dst_path, false); pfree(src_path); pfree(dst_path); diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index fa9e7ae0dff8c..68338369a187c 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -35,7 +35,18 @@ * a directory or a regular file is ignored. */ void -copydir(const char *fromdir, const char *todir, bool recurse, bool range_copy) +copydir(const char *fromdir, const char *todir, bool recurse) +{ + copydir_with_method(fromdir, todir, recurse, COPY_METHOD_COPY); +} + +/* + * copydir_with_method: copy a directory using a specific copy method + * + * Method is either a simple copy or a call to copy_file_range. + */ +void +copydir_with_method(const char *fromdir, const char *todir, bool recurse, CopyMethod copy_method) { DIR *xldir; struct dirent *xlde; @@ -69,12 +80,12 @@ copydir(const char *fromdir, const char *todir, bool recurse, bool range_copy) { /* recurse to handle subdirectories */ if (recurse) - copydir(fromfile, tofile, true, range_copy); + copydir_with_method(fromfile, tofile, true, copy_method); } else if (xlde_type == PGFILETYPE_REG) { - if (range_copy) - copy_file_reflink(fromfile, tofile); + if (copy_method == COPY_METHOD_COPY_FILE_RANGE) + copy_file_by_range(fromfile, tofile); else copy_file(fromfile, tofile); } @@ -225,7 +236,7 @@ copy_file(const char *fromfile, const char *tofile) * copy one file using copy_file_range to give filesystem a chance to do COW optimization */ void -copy_file_reflink(const char *fromfile, const char *tofile) +copy_file_by_range(const char *fromfile, const char *tofile) { #if defined(HAVE_COPY_FILE_RANGE) int srcfd; diff --git a/src/include/storage/copydir.h b/src/include/storage/copydir.h index d4d5a0063aeff..225846ab409d7 100644 --- a/src/include/storage/copydir.h +++ b/src/include/storage/copydir.h @@ -13,8 +13,18 @@ #ifndef COPYDIR_H #define COPYDIR_H -extern void copydir(const char *fromdir, const char *todir, bool recurse, bool range_copy); +/* + * Enumeration to denote copy modes. + */ +typedef enum CopyMethod +{ + COPY_METHOD_COPY, + COPY_METHOD_COPY_FILE_RANGE, +} CopyMethod; + +extern void copydir(const char *fromdir, const char *todir, bool recurse); +extern void copydir_with_method(const char *fromdir, const char *todir, bool recurse, CopyMethod copy_method); extern void copy_file(const char *fromfile, const char *tofile); -extern void copy_file_reflink(const char *fromfile, const char *tofile); +extern void copy_file_by_range(const char *fromfile, const char *tofile); #endif /* COPYDIR_H */ From 471a4859951b9c8e716908a68a89c8a3bccccba1 Mon Sep 17 00:00:00 2001 From: jodiew Date: Fri, 21 Mar 2025 00:07:58 +0000 Subject: [PATCH 3/3] Improve copy method name and fix whitespace error --- src/backend/commands/dbcommands.c | 6 +++--- src/include/storage/copydir.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 2013bd6c9aea1..1ef65c778d527 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -83,7 +83,7 @@ typedef enum CreateDBStrategy { CREATEDB_WAL_LOG, CREATEDB_FILE_COPY, - CREATEDB_FILE_COPY_RANGE, + CREATEDB_COPY_FILE_BY_RANGE, } CreateDBStrategy; typedef struct @@ -1025,7 +1025,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) else if (pg_strcasecmp(strategy, "file_copy") == 0) dbstrategy = CREATEDB_FILE_COPY; else if (pg_strcasecmp(strategy, "copy_file_range") == 0) - dbstrategy = CREATEDB_FILE_COPY_RANGE; + dbstrategy = CREATEDB_COPY_FILE_BY_RANGE; else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1512,7 +1512,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (dbstrategy == CREATEDB_WAL_LOG) CreateDatabaseUsingWalLog(src_dboid, dboid, src_deftablespace, dst_deftablespace); - else if (dbstrategy == CREATEDB_FILE_COPY_RANGE) + else if (dbstrategy == CREATEDB_COPY_FILE_BY_RANGE) CreateDatabaseUsingFileCopy(src_dboid, dboid, src_deftablespace, dst_deftablespace, COPY_METHOD_COPY_FILE_RANGE); else diff --git a/src/include/storage/copydir.h b/src/include/storage/copydir.h index 225846ab409d7..b2d4ba6429378 100644 --- a/src/include/storage/copydir.h +++ b/src/include/storage/copydir.h @@ -19,7 +19,7 @@ typedef enum CopyMethod { COPY_METHOD_COPY, - COPY_METHOD_COPY_FILE_RANGE, + COPY_METHOD_COPY_FILE_RANGE, } CopyMethod; extern void copydir(const char *fromdir, const char *todir, bool recurse);