diff --git a/src/backup.c b/src/backup.c index f6d3a958..364e8644 100644 --- a/src/backup.c +++ b/src/backup.c @@ -95,7 +95,6 @@ static void do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool backup_logs) { int i; - char database_path[MAXPGPATH]; char external_prefix[MAXPGPATH]; /* Temp value. Used as template */ char dst_backup_path[MAXPGPATH]; char label[1024]; @@ -265,10 +264,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool /* Update running backup meta with START LSN */ write_backup(¤t, true); - pgBackupGetPath(¤t, database_path, lengthof(database_path), - DATABASE_DIR); - pgBackupGetPath(¤t, external_prefix, lengthof(external_prefix), - EXTERNAL_DIR); + join_path_components(external_prefix, current.database_dir, EXTERNAL_DIR); /* initialize backup's file list */ backup_files_list = parray_new(); @@ -276,7 +272,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool /* start stream replication */ if (stream_wal) { - join_path_components(dst_backup_path, database_path, PG_XLOG_DIR); + join_path_components(dst_backup_path, current.database_dir, PG_XLOG_DIR); fio_mkdir(dst_backup_path, DIR_PERMISSION, FIO_BACKUP_HOST); start_WAL_streaming(backup_conn, dst_backup_path, &instance_config.conn_opt, @@ -441,7 +437,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool join_path_components(dirpath, temp, file->rel_path); } else - join_path_components(dirpath, database_path, file->rel_path); + join_path_components(dirpath, current.database_dir, file->rel_path); elog(VERBOSE, "Create directory '%s'", dirpath); fio_mkdir(dirpath, DIR_PERMISSION, FIO_BACKUP_HOST); @@ -475,7 +471,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool arg->nodeInfo = nodeInfo; arg->from_root = instance_config.pgdata; - arg->to_root = database_path; + arg->to_root = current.database_dir; arg->external_prefix = external_prefix; arg->external_dirs = external_dirs; arg->files_list = backup_files_list; @@ -552,7 +548,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool elog(ERROR, "Failed to find file \"%s\" in backup filelist.", XLOG_CONTROL_FILE); - set_min_recovery_point(pg_control, database_path, current.stop_lsn); + set_min_recovery_point(pg_control, current.database_dir, current.stop_lsn); } /* close and sync page header map */ @@ -609,7 +605,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool /* construct fullpath */ if (file->external_dir_num == 0) - join_path_components(to_fullpath, database_path, file->rel_path); + join_path_components(to_fullpath, current.database_dir, file->rel_path); else { char external_dst[MAXPGPATH]; @@ -726,8 +722,8 @@ pgdata_basic_setup(ConnectionOptions conn_opt, PGNodeInfo *nodeInfo) * Entry point of pg_probackup BACKUP subcommand. */ int -do_backup(time_t start_time, pgSetBackupParams *set_backup_params, - bool no_validate, bool no_sync, bool backup_logs) +do_backup(pgSetBackupParams *set_backup_params, + bool no_validate, bool no_sync, bool backup_logs) { PGconn *backup_conn = NULL; PGNodeInfo nodeInfo; @@ -736,13 +732,16 @@ do_backup(time_t start_time, pgSetBackupParams *set_backup_params, /* Initialize PGInfonode */ pgNodeInit(&nodeInfo); + /* Create backup directory and BACKUP_CONTROL_FILE */ + pgBackupCreateDir(¤t, backup_instance_path); + if (!instance_config.pgdata) elog(ERROR, "required parameter not specified: PGDATA " "(-D, --pgdata)"); /* Update backup status and other metainfo. */ current.status = BACKUP_STATUS_RUNNING; - current.start_time = start_time; + current.start_time = current.backup_id; StrNCpy(current.program_version, PROGRAM_VERSION, sizeof(current.program_version)); @@ -757,16 +756,13 @@ do_backup(time_t start_time, pgSetBackupParams *set_backup_params, elog(INFO, "Backup start, pg_probackup version: %s, instance: %s, backup ID: %s, backup mode: %s, " "wal mode: %s, remote: %s, compress-algorithm: %s, compress-level: %i", - PROGRAM_VERSION, instance_name, base36enc(start_time), pgBackupGetBackupMode(¤t), + PROGRAM_VERSION, instance_name, base36enc(current.backup_id), pgBackupGetBackupMode(¤t), current.stream ? "STREAM" : "ARCHIVE", IsSshProtocol() ? "true" : "false", deparse_compress_alg(current.compress_alg), current.compress_level); - /* Create backup directory and BACKUP_CONTROL_FILE */ - if (pgBackupCreateDir(¤t)) - elog(ERROR, "Cannot create backup directory"); if (!lock_backup(¤t, true, true)) elog(ERROR, "Cannot lock backup %s directory", - base36enc(current.start_time)); + base36enc(current.backup_id)); write_backup(¤t, true); /* set the error processing function for the backup process */ @@ -781,7 +777,7 @@ do_backup(time_t start_time, pgSetBackupParams *set_backup_params, backup_conn = pgdata_basic_setup(instance_config.conn_opt, &nodeInfo); if (current.from_replica) - elog(INFO, "Backup %s is going to be taken from standby", base36enc(start_time)); + elog(INFO, "Backup %s is going to be taken from standby", base36enc(current.backup_id)); /* TODO, print PostgreSQL full version */ //elog(INFO, "PostgreSQL version: %s", nodeInfo.server_version_str); diff --git a/src/catalog.c b/src/catalog.c index 4da6ff05..30215417 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -23,6 +23,7 @@ static pgBackup* get_closest_backup(timelineInfo *tlinfo); static pgBackup* get_oldest_backup(timelineInfo *tlinfo); static const char *backupModes[] = {"", "PAGE", "PTRACK", "DELTA", "FULL"}; static pgBackup *readBackupControlFile(const char *path); +static time_t create_backup_dir(pgBackup *backup, const char *backup_instance_path); static bool backup_lock_exit_hook_registered = false; static parray *lock_files = NULL; @@ -1136,12 +1137,18 @@ get_multi_timeline_parent(parray *backup_list, parray *tli_list, return NULL; } -/* create backup directory in $BACKUP_PATH */ -int -pgBackupCreateDir(pgBackup *backup) +/* Create backup directory in $BACKUP_PATH + * Note, that backup_id attribute is updated, + * so it is possible to get diffrent values in + * pgBackup.start_time and pgBackup.backup_id. + * It may be ok or maybe not, so it's up to the caller + * to fix it or let it be. + */ + +void +pgBackupCreateDir(pgBackup *backup, const char *backup_instance_path) { int i; - char path[MAXPGPATH]; parray *subdirs = parray_new(); parray_append(subdirs, pg_strdup(DATABASE_DIR)); @@ -1163,13 +1170,10 @@ pgBackupCreateDir(pgBackup *backup) free_dir_list(external_list); } - pgBackupGetPath(backup, path, lengthof(path), NULL); + backup->backup_id = create_backup_dir(backup, backup_instance_path); - if (!dir_is_empty(path, FIO_BACKUP_HOST)) - elog(ERROR, "backup destination is not empty \"%s\"", path); - - fio_mkdir(path, DIR_PERMISSION, FIO_BACKUP_HOST); - backup->root_dir = pgut_strdup(path); + if (backup->backup_id == 0) + elog(ERROR, "Cannot create backup directory: %s", strerror(errno)); backup->database_dir = pgut_malloc(MAXPGPATH); join_path_components(backup->database_dir, backup->root_dir, DATABASE_DIR); @@ -1180,11 +1184,47 @@ pgBackupCreateDir(pgBackup *backup) /* create directories for actual backup files */ for (i = 0; i < parray_num(subdirs); i++) { + char path[MAXPGPATH]; + join_path_components(path, backup->root_dir, parray_get(subdirs, i)); fio_mkdir(path, DIR_PERMISSION, FIO_BACKUP_HOST); } free_dir_list(subdirs); +} + +/* + * Create root directory for backup, + * update pgBackup.root_dir if directory creation was a success + */ +time_t +create_backup_dir(pgBackup *backup, const char *backup_instance_path) +{ + int attempts = 10; + + while (attempts--) + { + int rc; + char path[MAXPGPATH]; + time_t backup_id = time(NULL); + + join_path_components(path, backup_instance_path, base36enc(backup_id)); + + /* TODO: add wrapper for remote mode */ + rc = dir_create_dir(path, DIR_PERMISSION, true); + + if (rc == 0) + { + backup->root_dir = pgut_strdup(path); + return backup_id; + } + else + { + elog(WARNING, "Cannot create directory \"%s\": %s", path, strerror(errno)); + sleep(1); + } + } + return 0; } diff --git a/src/dir.c b/src/dir.c index 9d7e3701..7beede12 100644 --- a/src/dir.c +++ b/src/dir.c @@ -138,9 +138,13 @@ static TablespaceList external_remap_list = {NULL, NULL}; /* * Create directory, also create parent directories if necessary. + * In strict mode treat already existing directory as error. + * Return values: + * 0 - ok + * -1 - error (check errno) */ int -dir_create_dir(const char *dir, mode_t mode) +dir_create_dir(const char *dir, mode_t mode, bool strict) { char parent[MAXPGPATH]; @@ -149,14 +153,14 @@ dir_create_dir(const char *dir, mode_t mode) /* Create parent first */ if (access(parent, F_OK) == -1) - dir_create_dir(parent, mode); + dir_create_dir(parent, mode, false); /* Create directory */ if (mkdir(dir, mode) == -1) { - if (errno == EEXIST) /* already exist */ + if (errno == EEXIST && !strict) /* already exist */ return 0; - elog(ERROR, "cannot create directory \"%s\": %s", dir, strerror(errno)); + return -1; } return 0; diff --git a/src/init.c b/src/init.c index 431ea3b7..1ab6dc0f 100644 --- a/src/init.c +++ b/src/init.c @@ -34,15 +34,15 @@ do_init(void) } /* create backup catalog root directory */ - dir_create_dir(backup_path, DIR_PERMISSION); + dir_create_dir(backup_path, DIR_PERMISSION, false); /* create backup catalog data directory */ join_path_components(path, backup_path, BACKUPS_DIR); - dir_create_dir(path, DIR_PERMISSION); + dir_create_dir(path, DIR_PERMISSION, false); /* create backup catalog wal directory */ join_path_components(arclog_path_dir, backup_path, "wal"); - dir_create_dir(arclog_path_dir, DIR_PERMISSION); + dir_create_dir(arclog_path_dir, DIR_PERMISSION, false); elog(INFO, "Backup catalog '%s' successfully inited", backup_path); return 0; @@ -91,8 +91,8 @@ do_add_instance(InstanceConfig *instance) instance->name, instance->arclog_path); /* Create directory for data files of this specific instance */ - dir_create_dir(instance->backup_instance_path, DIR_PERMISSION); - dir_create_dir(instance->arclog_path, DIR_PERMISSION); + dir_create_dir(instance->backup_instance_path, DIR_PERMISSION, false); + dir_create_dir(instance->arclog_path, DIR_PERMISSION, false); /* * Write initial configuration file. diff --git a/src/merge.c b/src/merge.c index 05220a5d..3c51a1fa 100644 --- a/src/merge.c +++ b/src/merge.c @@ -639,7 +639,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup) makeExternalDirPathByNum(new_container, full_external_prefix, file->external_dir_num); join_path_components(dirpath, new_container, file->rel_path); - dir_create_dir(dirpath, DIR_PERMISSION); + dir_create_dir(dirpath, DIR_PERMISSION, false); } pg_atomic_init_flag(&file->lock); diff --git a/src/pg_probackup.c b/src/pg_probackup.c index e34195ae..90202fa8 100644 --- a/src/pg_probackup.c +++ b/src/pg_probackup.c @@ -799,8 +799,6 @@ main(int argc, char *argv[]) return do_init(); case BACKUP_CMD: { - time_t start_time = time(NULL); - current.stream = stream_wal; /* sanity */ @@ -808,7 +806,7 @@ main(int argc, char *argv[]) elog(ERROR, "required parameter not specified: BACKUP_MODE " "(-b, --backup-mode)"); - return do_backup(start_time, set_backup_params, no_validate, no_sync, backup_logs); + return do_backup(set_backup_params, no_validate, no_sync, backup_logs); } case RESTORE_CMD: return do_restore_or_validate(current.backup_id, diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 2b8253cb..46775ebf 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -781,7 +781,7 @@ extern char** commands_args; extern const char *pgdata_exclude_dir[]; /* in backup.c */ -extern int do_backup(time_t start_time, pgSetBackupParams *set_backup_params, +extern int do_backup(pgSetBackupParams *set_backup_params, bool no_validate, bool no_sync, bool backup_logs); extern void do_checkdb(bool need_amcheck, ConnectionOptions conn_opt, char *pgdata); @@ -916,7 +916,7 @@ extern void pgBackupGetPath2(const pgBackup *backup, char *path, size_t len, extern void pgBackupGetPathInInstance(const char *instance_name, const pgBackup *backup, char *path, size_t len, const char *subdir1, const char *subdir2); -extern int pgBackupCreateDir(pgBackup *backup); +extern void pgBackupCreateDir(pgBackup *backup, const char *backup_instance_path); extern void pgNodeInit(PGNodeInfo *node); extern void pgBackupInit(pgBackup *backup); extern void pgBackupFree(void *backup); @@ -980,7 +980,7 @@ extern void makeExternalDirPathByNum(char *ret_path, const char *pattern_path, const int dir_num); extern bool backup_contains_external(const char *dir, parray *dirs_list); -extern int dir_create_dir(const char *path, mode_t mode); +extern int dir_create_dir(const char *path, mode_t mode, bool strict); extern bool dir_is_empty(const char *path, fio_location location); extern bool fileExists(const char *path, fio_location location); diff --git a/src/utils/file.c b/src/utils/file.c index b29a6707..c5c962ed 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -1048,7 +1048,7 @@ int fio_mkdir(char const* path, int mode, fio_location location) } else { - return dir_create_dir(path, mode); + return dir_create_dir(path, mode, false); } } @@ -2648,7 +2648,7 @@ void fio_communicate(int in, int out) break; case FIO_MKDIR: /* Create directory */ hdr.size = 0; - hdr.arg = dir_create_dir(buf, hdr.arg); + hdr.arg = dir_create_dir(buf, hdr.arg, false); IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); break; case FIO_CHMOD: /* Change file mode */ diff --git a/tests/backup.py b/tests/backup.py index de53c207..66bc5d32 100644 --- a/tests/backup.py +++ b/tests/backup.py @@ -2921,12 +2921,14 @@ class BackupTest(ProbackupTest, unittest.TestCase): try: self.backup_node( - backup_dir, 'node', node, - data_dir='{0}'.format(datadir), return_id=False) + backup_dir, 'node', node, data_dir='{0}'.format(datadir)) except: pass - self.backup_node(backup_dir, 'node', node, options=['--stream']) + out = self.backup_node(backup_dir, 'node', node, options=['--stream'], return_id=False) + + # it is a bit racy + self.assertIn("WARNING: Cannot create directory", out) # Clean after yourself self.del_test_dir(module_name, fname)