From d2c9fceca0237edab216d02b7a4a9d07a79e3ffe Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Wed, 20 Dec 2017 17:57:01 +0300 Subject: [PATCH] Add base36enc() and base36enc_dup(). base36enc() - returns static text base36enc_dup() - returns duplicated text, which should be released by a user --- src/backup.c | 12 ++++-------- src/catalog.c | 21 ++++++--------------- src/delete.c | 5 +---- src/parsexlog.c | 8 +++----- src/pg_probackup.c | 6 +----- src/pg_probackup.h | 3 ++- src/restore.c | 33 ++++++++++++++++----------------- src/show.c | 6 +----- src/util.c | 25 ++++++++++++++++++++++--- src/validate.c | 21 +++++++++++++-------- 10 files changed, 69 insertions(+), 71 deletions(-) diff --git a/src/backup.c b/src/backup.c index cf267878..291a8cc6 100644 --- a/src/backup.c +++ b/src/backup.c @@ -1541,23 +1541,18 @@ pg_stop_backup(pgBackup *backup) { const char *params[1]; char name[1024]; - char *backup_id; - - backup_id = base36enc(backup->start_time); if (!from_replica) snprintf(name, lengthof(name), "pg_probackup, backup_id %s", - backup_id); + base36enc(backup->start_time)); else snprintf(name, lengthof(name), "pg_probackup, backup_id %s. Replica Backup", - backup_id); + base36enc(backup->start_time)); params[0] = name; res = pgut_execute(conn, "SELECT pg_create_restore_point($1)", 1, params, true); PQclear(res); - - pfree(backup_id); } /* @@ -1845,7 +1840,8 @@ backup_cleanup(bool fatal, void *userdata) */ if (current.status == BACKUP_STATUS_RUNNING && current.end_time == 0) { - elog(INFO, "Backup %s is running, setting its status to ERROR", base36enc(current.start_time)); + elog(INFO, "Backup %s is running, setting its status to ERROR", + base36enc(current.start_time)); current.end_time = time(NULL); current.status = BACKUP_STATUS_ERROR; pgBackupWriteBackupControlFile(¤t); diff --git a/src/catalog.c b/src/catalog.c index de61b7b1..0e9b4de2 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -436,12 +436,7 @@ pgBackupWriteControl(FILE *out, pgBackup *backup) /* 'parent_backup' is set if it is incremental backup */ if (backup->parent_backup != 0) - { - char *parent_backup = base36enc(backup->parent_backup); - - fprintf(out, "parent-backup-id = '%s'\n", parent_backup); - free(parent_backup); - } + fprintf(out, "parent-backup-id = '%s'\n", base36enc(backup->parent_backup)); } /* create BACKUP_CONTROL_FILE */ @@ -669,21 +664,17 @@ void pgBackupGetPath2(const pgBackup *backup, char *path, size_t len, const char *subdir1, const char *subdir2) { - char *datetime; - - datetime = base36enc(backup->start_time); - /* If "subdir1" is NULL do not check "subdir2" */ if (!subdir1) - snprintf(path, len, "%s/%s", backup_instance_path, datetime); + snprintf(path, len, "%s/%s", backup_instance_path, + base36enc(backup->start_time)); else if (!subdir2) - snprintf(path, len, "%s/%s/%s", backup_instance_path, datetime, subdir1); + snprintf(path, len, "%s/%s/%s", backup_instance_path, + base36enc(backup->start_time), subdir1); /* "subdir1" and "subdir2" is not NULL */ else snprintf(path, len, "%s/%s/%s/%s", backup_instance_path, - datetime, subdir1, subdir2); - - free(datetime); + base36enc(backup->start_time), subdir1, subdir2); make_native_path(path); } diff --git a/src/delete.c b/src/delete.c index 536a65d4..8a2a4ba4 100644 --- a/src/delete.c +++ b/src/delete.c @@ -224,7 +224,6 @@ static int pgBackupDeleteFiles(pgBackup *backup) { size_t i; - char *backup_id; char path[MAXPGPATH]; char timestamp[100]; parray *files; @@ -235,11 +234,9 @@ pgBackupDeleteFiles(pgBackup *backup) if (backup->status == BACKUP_STATUS_DELETED) return 0; - backup_id = base36enc(backup->start_time); time2iso(timestamp, lengthof(timestamp), backup->recovery_time); - elog(INFO, "delete: %s %s", backup_id, timestamp); - free(backup_id); + elog(INFO, "delete: %s %s", base36enc(backup->start_time), timestamp); /* * Update STATUS to BACKUP_STATUS_DELETING in preparation for the case which diff --git a/src/parsexlog.c b/src/parsexlog.c index a758e210..79dc57fc 100644 --- a/src/parsexlog.c +++ b/src/parsexlog.c @@ -290,7 +290,7 @@ validate_wal(pgBackup *backup, TimeLineID tli) { XLogRecPtr startpoint = backup->start_lsn; - char *backup_id; + const char *backup_id; XLogRecord *record; XLogReaderState *xlogreader; char *errormsg; @@ -329,11 +329,9 @@ validate_wal(pgBackup *backup, else validate_backup_wal_from_start_to_stop(backup, (char *) archivedir, tli); - free(backup_id); - if (backup->status == BACKUP_STATUS_CORRUPT) { - elog(WARNING, "Backup %s WAL segments are corrupted", base36enc(backup->start_time)); + elog(WARNING, "Backup %s WAL segments are corrupted", backup_id); return; } /* @@ -343,7 +341,7 @@ validate_wal(pgBackup *backup, if (!TransactionIdIsValid(target_xid) && target_time == 0) { /* Recovery target is not given so exit */ - elog(INFO, "Backup %s WAL segments are valid", base36enc(backup->start_time)); + elog(INFO, "Backup %s WAL segments are valid", backup_id); return; } diff --git a/src/pg_probackup.c b/src/pg_probackup.c index 5e6bd99f..7665153c 100644 --- a/src/pg_probackup.c +++ b/src/pg_probackup.c @@ -423,21 +423,17 @@ main(int argc, char *argv[]) return do_init(); case BACKUP: { - char *backup_id; const char *backup_mode; time_t start_time; start_time = time(NULL); - backup_id = base36enc(start_time); backup_mode = deparse_backup_mode(current.backup_mode); elog(INFO, "Backup start, pg_probackup version: %s, backup ID: %s, backup mode: %s, instance: %s, stream: %s, remote: %s", - PROGRAM_VERSION, backup_id, backup_mode, instance_name, + PROGRAM_VERSION, base36enc(start_time), backup_mode, instance_name, current.stream ? "true" : "false", is_remote_backup ? "true" : "false"); elog_file(INFO, "command: %s", command); - pfree(backup_id); - return do_backup(start_time); } case RESTORE: diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 4ddc0fe9..72d1e437 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -461,7 +461,8 @@ extern const char *status2str(BackupStatus status); extern void remove_trailing_space(char *buf, int comment_mark); extern void remove_not_digit(char *buf, size_t len, const char *str); extern uint32 get_data_checksum_version(bool safe); -extern char *base36enc(long unsigned int value); +extern const char *base36enc(long unsigned int value); +extern char *base36enc_dup(long unsigned int value); extern long unsigned int base36dec(const char *text); extern uint64 get_system_identifier(char *pgdata); extern uint64 get_remote_system_identifier(PGconn *conn); diff --git a/src/restore.c b/src/restore.c index 0c7fa073..dd19fea2 100644 --- a/src/restore.c +++ b/src/restore.c @@ -195,8 +195,8 @@ do_restore_or_validate(time_t target_backup_id, { if (current_backup->status != BACKUP_STATUS_OK) elog(ERROR, "base backup %s for given backup %s is in %s status", - base36enc(current_backup->start_time), - base36enc(dest_backup->start_time), + base36enc_dup(current_backup->start_time), + base36enc_dup(dest_backup->start_time), status2str(current_backup->status)); else { @@ -264,10 +264,20 @@ do_restore_or_validate(time_t target_backup_id, continue; else { + char *backup_id, + *corrupted_backup_id; + backup->status = BACKUP_STATUS_ORPHAN; pgBackupWriteBackupControlFile(backup); + + backup_id = base36enc_dup(backup->start_time); + corrupted_backup_id = base36enc_dup(corrupted_backup->start_time); + elog(WARNING, "Backup %s is orphaned because his parent %s is corrupted", - base36enc(backup->start_time), base36enc(corrupted_backup->start_time)); + backup_id, corrupted_backup_id); + + free(backup_id); + free(corrupted_backup_id); } } } @@ -409,13 +419,7 @@ restore_backup(pgBackup *backup) parray_free(files); if (LOG_LEVEL_CONSOLE <= LOG || LOG_LEVEL_FILE <= LOG) - { - char *backup_id; - - backup_id = base36enc(backup->start_time); - elog(LOG, "restore %s backup completed", backup_id); - free(backup_id); - } + elog(LOG, "restore %s backup completed", base36enc(backup->start_time)); } /* @@ -635,13 +639,8 @@ check_tablespace_mapping(pgBackup *backup) read_tablespace_map(links, this_backup_path); if (LOG_LEVEL_CONSOLE <= LOG || LOG_LEVEL_FILE <= LOG) - { - char *backup_id; - - backup_id = base36enc(backup->start_time); - elog(LOG, "check tablespace directories of backup %s", backup_id); - pfree(backup_id); - } + elog(LOG, "check tablespace directories of backup %s", + base36enc(backup->start_time)); /* 1 - each OLDDIR must have an entry in tablespace_map file (links) */ for (cell = tablespace_dirs.head; cell; cell = cell->next) diff --git a/src/show.c b/src/show.c index 29418aa3..733a456e 100644 --- a/src/show.c +++ b/src/show.c @@ -234,7 +234,6 @@ show_backup_list(FILE *out, parray *backup_list) { pgBackup *backup = parray_get(backup_list, i); TimeLineID parent_tli; - char *backup_id; char timestamp[100] = "----"; char duration[20] = "----"; char data_bytes_str[10] = "----"; @@ -255,12 +254,11 @@ show_backup_list(FILE *out, parray *backup_list) /* Get parent timeline before printing */ parent_tli = get_parent_tli(backup->tli); - backup_id = base36enc(backup->start_time); fprintf(out, " %-11s %-8s %-6s %-22s %-6s %-7s %3d / %-3d %5s %6s %2X/%-8X %2X/%-8X %-8s\n", instance_name, (backup->server_version[0] ? backup->server_version : "----"), - backup_id, + base36enc(backup->start_time), timestamp, pgBackupGetBackupMode(backup), backup->stream ? "STREAM": "ARCHIVE", @@ -273,8 +271,6 @@ show_backup_list(FILE *out, parray *backup_list) (uint32) (backup->stop_lsn >> 32), (uint32) backup->stop_lsn, status2str(backup->status)); - - free(backup_id); } } diff --git a/src/util.c b/src/util.c index b4cfdd6b..e254baee 100644 --- a/src/util.c +++ b/src/util.c @@ -14,10 +14,29 @@ #include "storage/bufpage.h" -char * +const char * base36enc(long unsigned int value) { - char base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + const char base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + /* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */ + static char buffer[14]; + unsigned int offset = sizeof(buffer); + + buffer[--offset] = '\0'; + do { + buffer[--offset] = base36[value % 36]; + } while (value /= 36); + + return buffer; +} + +/* + * Same as base36enc(), but the result must be released by the user. + */ +char * +base36enc_dup(long unsigned int value) +{ + const char base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; /* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */ char buffer[14]; unsigned int offset = sizeof(buffer); @@ -27,7 +46,7 @@ base36enc(long unsigned int value) buffer[--offset] = base36[value % 36]; } while (value /= 36); - return strdup(&buffer[offset]); /* warning: this must be free-d by the user */ + return strdup(&buffer[offset]); } long unsigned int diff --git a/src/validate.c b/src/validate.c index 3a60efd5..2a04471b 100644 --- a/src/validate.c +++ b/src/validate.c @@ -31,7 +31,7 @@ typedef struct void pgBackupValidate(pgBackup *backup) { - char *backup_id_string; + const char *backup_id_string; char base_path[MAXPGPATH]; char path[MAXPGPATH]; parray *files; @@ -100,7 +100,6 @@ pgBackupValidate(pgBackup *backup) elog(WARNING, "Backup %s data files are corrupted", backup_id_string); else elog(INFO, "Backup %s data files are valid", backup_id_string); - free(backup_id_string); } /* @@ -263,11 +262,9 @@ do_validate_instance(void) /* Valiate each backup along with its xlog files. */ for (i = 0; i < parray_num(backups); i++) { - char *backup_id; pgBackup *base_full_backup = NULL; current_backup = (pgBackup *) parray_get(backups, i); - backup_id = base36enc(current_backup->start_time); if (current_backup->backup_mode != BACKUP_MODE_FULL) { @@ -294,7 +291,7 @@ do_validate_instance(void) { if (base_full_backup == NULL) elog(ERROR, "Valid full backup for backup %s is not found.", - backup_id); + base36enc(current_backup->start_time)); /* Validate corresponding WAL files */ validate_wal(current_backup, arclog_path, 0, 0, base_full_backup->tli); @@ -313,15 +310,23 @@ do_validate_instance(void) continue; else { + char *backup_id, + *current_backup_id; + backup->status = BACKUP_STATUS_ORPHAN; pgBackupWriteBackupControlFile(backup); + + backup_id = base36enc_dup(backup->start_time); + current_backup_id = base36enc_dup(current_backup->start_time); + elog(WARNING, "Backup %s is orphaned because his parent %s is corrupted", - base36enc(backup->start_time), base36enc(current_backup->start_time)); + backup_id, current_backup_id); + + free(backup_id); + free(current_backup_id); } } } - - free(backup_id); } /* cleanup */