Skip to content

Commit 87dd3f2

Browse files
committed
base36enc(backup->backup_id) -> backup_id_of(backup)
Here we assume backup_id == start_time. It is really so at the moment, but could change in future. Well, it almost always same. Sometime backup_id is set while start_time is not set yet (backup creation). And we had to fix places where start_time were changed without change of backup_id.
1 parent 44fef88 commit 87dd3f2

File tree

7 files changed

+31
-12
lines changed

7 files changed

+31
-12
lines changed

src/backup.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,7 @@ do_backup(InstanceState *instanceState, pgSetBackupParams *set_backup_params,
768768

769769
/* Update backup status and other metainfo. */
770770
current.status = BACKUP_STATUS_RUNNING;
771+
/* XXX BACKUP_ID change it when backup_id wouldn't match start_time */
771772
current.start_time = current.backup_id;
772773

773774
strlcpy(current.program_version, PROGRAM_VERSION,
@@ -778,13 +779,13 @@ do_backup(InstanceState *instanceState, pgSetBackupParams *set_backup_params,
778779

779780
elog(INFO, "Backup start, pg_probackup version: %s, instance: %s, backup ID: %s, backup mode: %s, "
780781
"wal mode: %s, remote: %s, compress-algorithm: %s, compress-level: %i",
781-
PROGRAM_VERSION, instanceState->instance_name, base36enc(current.backup_id), pgBackupGetBackupMode(&current, false),
782+
PROGRAM_VERSION, instanceState->instance_name, backup_id_of(&current), pgBackupGetBackupMode(&current, false),
782783
current.stream ? "STREAM" : "ARCHIVE", IsSshProtocol() ? "true" : "false",
783784
deparse_compress_alg(current.compress_alg), current.compress_level);
784785

785786
if (!lock_backup(&current, true, true))
786787
elog(ERROR, "Cannot lock backup %s directory",
787-
base36enc(current.backup_id));
788+
backup_id_of(&current));
788789
write_backup(&current, true);
789790

790791
/* set the error processing function for the backup process */
@@ -799,7 +800,7 @@ do_backup(InstanceState *instanceState, pgSetBackupParams *set_backup_params,
799800
backup_conn = pgdata_basic_setup(instance_config.conn_opt, &nodeInfo);
800801

801802
if (current.from_replica)
802-
elog(INFO, "Backup %s is going to be taken from standby", base36enc(current.backup_id));
803+
elog(INFO, "Backup %s is going to be taken from standby", backup_id_of(&current));
803804

804805
/* TODO, print PostgreSQL full version */
805806
//elog(INFO, "PostgreSQL version: %s", nodeInfo.server_version_str);

src/catalog.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ lock_backup(pgBackup *backup, bool strict, bool exclusive)
275275

276276
/* save lock metadata for later unlocking */
277277
lock = pgut_malloc(sizeof(LockInfo));
278-
snprintf(lock->backup_id, 10, "%s", base36enc(backup->backup_id));
278+
snprintf(lock->backup_id, 10, "%s", backup_id_of(backup));
279279
snprintf(lock->backup_dir, MAXPGPATH, "%s", backup->root_dir);
280280
lock->exclusive = exclusive;
281281

@@ -966,6 +966,9 @@ catalog_get_backup_list(InstanceState *instanceState, time_t requested_backup_id
966966
backup = pgut_new0(pgBackup);
967967
pgBackupInit(backup);
968968
backup->start_time = base36dec(data_ent->d_name);
969+
/* XXX BACKUP_ID change it when backup_id wouldn't match start_time */
970+
Assert(backup->backup_id == 0 || backup->backup_id == backup->start_time);
971+
backup->backup_id = backup->start_time;
969972
}
970973
else if (strcmp(backup_id_of(backup), data_ent->d_name) != 0)
971974
{
@@ -983,7 +986,6 @@ catalog_get_backup_list(InstanceState *instanceState, time_t requested_backup_id
983986
init_header_map(backup);
984987

985988
/* TODO: save encoded backup id */
986-
backup->backup_id = backup->start_time;
987989
if (requested_backup_id != INVALID_BACKUP_ID
988990
&& requested_backup_id != backup->start_time)
989991
{
@@ -1454,7 +1456,7 @@ pgBackupInitDir(pgBackup *backup, const char *backup_instance_path)
14541456
if (create_backup_dir(backup, backup_instance_path) != 0)
14551457
{
14561458
/* Clear backup_id as indication of error */
1457-
backup->backup_id = INVALID_BACKUP_ID;
1459+
reset_backup_id(backup);
14581460
return;
14591461
}
14601462

@@ -1506,7 +1508,7 @@ create_backup_dir(pgBackup *backup, const char *backup_instance_path)
15061508
int rc;
15071509
char path[MAXPGPATH];
15081510

1509-
join_path_components(path, backup_instance_path, base36enc(backup->backup_id));
1511+
join_path_components(path, backup_instance_path, backup_id_of(backup));
15101512

15111513
/* TODO: add wrapper for remote mode */
15121514
rc = dir_create_dir(path, DIR_PERMISSION, true);
@@ -2252,7 +2254,7 @@ pin_backup(pgBackup *target_backup, pgSetBackupParams *set_backup_params)
22522254
/* sanity, backup must have positive recovery-time */
22532255
if (target_backup->recovery_time <= 0)
22542256
elog(ERROR, "Failed to set 'expire-time' for backup %s: invalid 'recovery-time'",
2255-
base36enc(target_backup->backup_id));
2257+
backup_id_of(target_backup));
22562258

22572259
/* Pin comes from ttl */
22582260
if (set_backup_params->ttl > 0)
@@ -2714,6 +2716,9 @@ readBackupControlFile(const char *path)
27142716
pgBackupFree(backup);
27152717
return NULL;
27162718
}
2719+
/* XXX BACKUP_ID change it when backup_id wouldn't match start_time */
2720+
Assert(backup->backup_id == 0 || backup->backup_id == backup->start_time);
2721+
backup->backup_id = backup->start_time;
27172722

27182723
if (backup_mode)
27192724
{

src/dir.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,7 @@ check_tablespace_mapping(pgBackup *backup, bool incremental, bool force, bool pg
11151115
*/
11161116
if (tablespace_dirs.head != NULL)
11171117
elog(ERROR, "Backup %s has no tablespaceses, nothing to remap "
1118-
"via \"--tablespace-mapping\" option", base36enc(backup->backup_id));
1118+
"via \"--tablespace-mapping\" option", backup_id_of(backup));
11191119
return NoTblspc;
11201120
}
11211121

src/merge.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,8 @@ merge_chain(InstanceState *instanceState,
874874

875875
full_backup->status = BACKUP_STATUS_OK;
876876
full_backup->start_time = full_backup->merge_dest_backup;
877+
/* XXX BACKUP_ID change it when backup_id wouldn't match start_time */
878+
full_backup->backup_id = full_backup->start_time;
877879
full_backup->merge_dest_backup = INVALID_BACKUP_ID;
878880
write_backup(full_backup, true);
879881
/* Critical section end */

src/pg_probackup.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,7 @@ extern parray *get_dbOid_exclude_list(pgBackup *backup, parray *datname_list,
887887
PartialRestoreType partial_restore_type);
888888

889889
extern const char* backup_id_of(pgBackup *backup);
890+
extern void reset_backup_id(pgBackup *backup);
890891

891892
extern parray *get_backup_filelist(pgBackup *backup, bool strict);
892893
extern parray *read_timeline_history(const char *arclog_path, TimeLineID targetTLI, bool strict);

src/util.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,19 @@ datapagemap_print_debug(datapagemap_t *map)
565565
const char*
566566
backup_id_of(pgBackup *backup)
567567
{
568+
/* Change this Assert when backup_id will not be bound to start_time */
569+
Assert(backup->backup_id == backup->start_time || backup->start_time == 0);
570+
568571
if (backup->backup_id_encoded[0] == '\x00')
569572
{
570-
base36enc_to(backup->start_time, backup->backup_id_encoded);
573+
base36enc_to(backup->backup_id, backup->backup_id_encoded);
571574
}
572575
return backup->backup_id_encoded;
573576
}
577+
578+
void
579+
reset_backup_id(pgBackup *backup)
580+
{
581+
backup->backup_id = INVALID_BACKUP_ID;
582+
memset(backup->backup_id_encoded, 0, sizeof(backup->backup_id_encoded));
583+
}

src/validate.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ validate_tablespace_map(pgBackup *backup, bool no_validate)
734734
if (!fileExists(map_path, FIO_BACKUP_HOST))
735735
elog(ERROR, "Tablespace map is missing: \"%s\", "
736736
"probably backup %s is corrupt, validate it",
737-
map_path, base36enc(backup->backup_id));
737+
map_path, backup_id_of(backup));
738738

739739
/* check tablespace map checksumms */
740740
if (!no_validate)
@@ -744,7 +744,7 @@ validate_tablespace_map(pgBackup *backup, bool no_validate)
744744
if ((*tablespace_map)->crc != crc)
745745
elog(ERROR, "Invalid CRC of tablespace map file \"%s\" : %X. Expected %X, "
746746
"probably backup %s is corrupt, validate it",
747-
map_path, crc, (*tablespace_map)->crc, base36enc(backup->backup_id));
747+
map_path, crc, (*tablespace_map)->crc, backup_id_of(backup));
748748
}
749749

750750
pgFileFree(dummy);

0 commit comments

Comments
 (0)