Skip to content

Commit 6c21833

Browse files
committed
[Issue #272] fix DST time skew, store backup metadata timestamps in UTC
1 parent 1cf233d commit 6c21833

File tree

9 files changed

+78
-55
lines changed

9 files changed

+78
-55
lines changed

src/backup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync, bool
133133
pg_ptrack_clear(backup_conn, nodeInfo->ptrack_version_num);
134134

135135
/* notify start of backup to PostgreSQL server */
136-
time2iso(label, lengthof(label), current.start_time);
136+
time2iso(label, lengthof(label), current.start_time, false);
137137
strncat(label, " with pg_probackup", lengthof(label) -
138138
strlen(" with pg_probackup"));
139139

src/catalog.c

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ catalog_get_backup_list(const char *instance_name, time_t requested_backup_id)
789789
}
790790
else if (strcmp(base36enc(backup->start_time), data_ent->d_name) != 0)
791791
{
792-
elog(VERBOSE, "backup ID in control file \"%s\" doesn't match name of the backup folder \"%s\"",
792+
elog(WARNING, "backup ID in control file \"%s\" doesn't match name of the backup folder \"%s\"",
793793
base36enc(backup->start_time), backup_conf_path);
794794
}
795795

@@ -1952,7 +1952,7 @@ pin_backup(pgBackup *target_backup, pgSetBackupParams *set_backup_params)
19521952
{
19531953
char expire_timestamp[100];
19541954

1955-
time2iso(expire_timestamp, lengthof(expire_timestamp), target_backup->expire_time);
1955+
time2iso(expire_timestamp, lengthof(expire_timestamp), target_backup->expire_time, false);
19561956
elog(INFO, "Backup %s is pinned until '%s'", base36enc(target_backup->start_time),
19571957
expire_timestamp);
19581958
}
@@ -2003,7 +2003,7 @@ add_note(pgBackup *target_backup, char *note)
20032003
* Write information about backup.in to stream "out".
20042004
*/
20052005
void
2006-
pgBackupWriteControl(FILE *out, pgBackup *backup)
2006+
pgBackupWriteControl(FILE *out, pgBackup *backup, bool utc)
20072007
{
20082008
char timestamp[100];
20092009

@@ -2035,27 +2035,27 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
20352035
(uint32) (backup->stop_lsn >> 32),
20362036
(uint32) backup->stop_lsn);
20372037

2038-
time2iso(timestamp, lengthof(timestamp), backup->start_time);
2038+
time2iso(timestamp, lengthof(timestamp), backup->start_time, utc);
20392039
fio_fprintf(out, "start-time = '%s'\n", timestamp);
20402040
if (backup->merge_time > 0)
20412041
{
2042-
time2iso(timestamp, lengthof(timestamp), backup->merge_time);
2042+
time2iso(timestamp, lengthof(timestamp), backup->merge_time, utc);
20432043
fio_fprintf(out, "merge-time = '%s'\n", timestamp);
20442044
}
20452045
if (backup->end_time > 0)
20462046
{
2047-
time2iso(timestamp, lengthof(timestamp), backup->end_time);
2047+
time2iso(timestamp, lengthof(timestamp), backup->end_time, utc);
20482048
fio_fprintf(out, "end-time = '%s'\n", timestamp);
20492049
}
20502050
fio_fprintf(out, "recovery-xid = " XID_FMT "\n", backup->recovery_xid);
20512051
if (backup->recovery_time > 0)
20522052
{
2053-
time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
2053+
time2iso(timestamp, lengthof(timestamp), backup->recovery_time, utc);
20542054
fio_fprintf(out, "recovery-time = '%s'\n", timestamp);
20552055
}
20562056
if (backup->expire_time > 0)
20572057
{
2058-
time2iso(timestamp, lengthof(timestamp), backup->expire_time);
2058+
time2iso(timestamp, lengthof(timestamp), backup->expire_time, utc);
20592059
fio_fprintf(out, "expire-time = '%s'\n", timestamp);
20602060
}
20612061

@@ -2109,29 +2109,29 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
21092109
void
21102110
write_backup(pgBackup *backup, bool strict)
21112111
{
2112-
FILE *fp_out = NULL;
2112+
FILE *fp = NULL;
21132113
char path[MAXPGPATH];
21142114
char path_temp[MAXPGPATH];
21152115
char buf[8192];
21162116

21172117
join_path_components(path, backup->root_dir, BACKUP_CONTROL_FILE);
21182118
snprintf(path_temp, sizeof(path_temp), "%s.tmp", path);
21192119

2120-
fp_out = fopen(path_temp, PG_BINARY_W);
2121-
if (fp_out == NULL)
2120+
fp = fopen(path_temp, PG_BINARY_W);
2121+
if (fp == NULL)
21222122
elog(ERROR, "Cannot open control file \"%s\": %s",
21232123
path_temp, strerror(errno));
21242124

21252125
if (chmod(path_temp, FILE_PERMISSION) == -1)
21262126
elog(ERROR, "Cannot change mode of \"%s\": %s", path_temp,
21272127
strerror(errno));
21282128

2129-
setvbuf(fp_out, buf, _IOFBF, sizeof(buf));
2129+
setvbuf(fp, buf, _IOFBF, sizeof(buf));
21302130

2131-
pgBackupWriteControl(fp_out, backup);
2131+
pgBackupWriteControl(fp, backup, true);
21322132

21332133
/* Ignore 'out of space' error in lax mode */
2134-
if (fflush(fp_out) != 0)
2134+
if (fflush(fp) != 0)
21352135
{
21362136
int elevel = ERROR;
21372137
int save_errno = errno;
@@ -2144,17 +2144,18 @@ write_backup(pgBackup *backup, bool strict)
21442144

21452145
if (!strict && (save_errno == ENOSPC))
21462146
{
2147-
fclose(fp_out);
2147+
fclose(fp);
2148+
fio_unlink(path_temp, FIO_BACKUP_HOST);
21482149
return;
21492150
}
21502151
}
21512152

2152-
if (fsync(fileno(fp_out)) < 0)
2153-
elog(ERROR, "Cannot sync control file \"%s\": %s",
2153+
if (fclose(fp) != 0)
2154+
elog(ERROR, "Cannot close control file \"%s\": %s",
21542155
path_temp, strerror(errno));
21552156

2156-
if (fclose(fp_out) != 0)
2157-
elog(ERROR, "Cannot close control file \"%s\": %s",
2157+
if (fio_sync(path_temp, FIO_BACKUP_HOST) < 0)
2158+
elog(ERROR, "Cannot sync control file \"%s\": %s",
21582159
path_temp, strerror(errno));
21592160

21602161
if (rename(path_temp, path) < 0)

src/delete.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ do_retention_internal(parray *backup_list, parray *to_keep_list, parray *to_purg
316316
(backup->expire_time > current_time))
317317
{
318318
char expire_timestamp[100];
319-
time2iso(expire_timestamp, lengthof(expire_timestamp), backup->expire_time);
319+
time2iso(expire_timestamp, lengthof(expire_timestamp), backup->expire_time, false);
320320

321321
elog(LOG, "Backup %s is pinned until '%s', retain",
322322
base36enc(backup->start_time), expire_timestamp);
@@ -740,7 +740,7 @@ delete_backup_files(pgBackup *backup)
740740
return;
741741
}
742742

743-
time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
743+
time2iso(timestamp, lengthof(timestamp), backup->recovery_time, false);
744744

745745
elog(INFO, "Delete: %s %s",
746746
base36enc(backup->start_time), timestamp);

src/parsexlog.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ validate_wal(pgBackup *backup, const char *archivedir,
480480
last_rec.rec_xid = backup->recovery_xid;
481481
last_rec.rec_lsn = backup->stop_lsn;
482482

483-
time2iso(last_timestamp, lengthof(last_timestamp), backup->recovery_time);
483+
time2iso(last_timestamp, lengthof(last_timestamp), backup->recovery_time, false);
484484

485485
if ((TransactionIdIsValid(target_xid) && target_xid == last_rec.rec_xid)
486486
|| (target_time != 0 && backup->recovery_time >= target_time)
@@ -493,7 +493,7 @@ validate_wal(pgBackup *backup, const char *archivedir,
493493
InvalidXLogRecPtr, true, validateXLogRecord, &last_rec, true);
494494
if (last_rec.rec_time > 0)
495495
time2iso(last_timestamp, lengthof(last_timestamp),
496-
timestamptz_to_time_t(last_rec.rec_time));
496+
timestamptz_to_time_t(last_rec.rec_time), false);
497497

498498
/* There are all needed WAL records */
499499
if (all_wal)
@@ -508,7 +508,7 @@ validate_wal(pgBackup *backup, const char *archivedir,
508508
(uint32) (last_rec.rec_lsn >> 32), (uint32) last_rec.rec_lsn);
509509

510510
if (target_time > 0)
511-
time2iso(target_timestamp, lengthof(target_timestamp), target_time);
511+
time2iso(target_timestamp, lengthof(target_timestamp), target_time, false);
512512
if (TransactionIdIsValid(target_xid) && target_time != 0)
513513
elog(ERROR, "Not enough WAL records to time %s and xid " XID_FMT,
514514
target_timestamp, target_xid);

src/pg_probackup.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,9 @@ struct pgBackup
394394
TimeLineID tli; /* timeline of start and stop backup lsns */
395395
XLogRecPtr start_lsn; /* backup's starting transaction log location */
396396
XLogRecPtr stop_lsn; /* backup's finishing transaction log location */
397-
time_t start_time; /* since this moment backup has status
398-
* BACKUP_STATUS_RUNNING */
399-
time_t merge_dest_backup; /* start_time of incremental backup,
400-
* this backup is merging with.
397+
time_t start_time; /* UTC time of backup creation */
398+
time_t merge_dest_backup; /* start_time of incremental backup with
399+
* which this backup is merging with.
401400
* Only available for FULL backups
402401
* with MERGING or MERGED statuses */
403402
time_t merge_time; /* the moment when merge was started or 0 */
@@ -892,7 +891,7 @@ extern void do_set_backup(const char *instance_name, time_t backup_id,
892891
extern void pin_backup(pgBackup *target_backup,
893892
pgSetBackupParams *set_backup_params);
894893
extern void add_note(pgBackup *target_backup, char *note);
895-
extern void pgBackupWriteControl(FILE *out, pgBackup *backup);
894+
extern void pgBackupWriteControl(FILE *out, pgBackup *backup, bool utc);
896895
extern void write_backup_filelist(pgBackup *backup, parray *files,
897896
const char *root, parray *external_list, bool sync);
898897

@@ -1081,7 +1080,7 @@ extern void set_min_recovery_point(pgFile *file, const char *backup_path,
10811080
extern void copy_pgcontrol_file(const char *from_fullpath, fio_location from_location,
10821081
const char *to_fullpath, fio_location to_location, pgFile *file);
10831082

1084-
extern void time2iso(char *buf, size_t len, time_t time);
1083+
extern void time2iso(char *buf, size_t len, time_t time, bool utc);
10851084
extern const char *status2str(BackupStatus status);
10861085
extern BackupStatus str2status(const char *status);
10871086
extern const char *base36enc(long unsigned int value);

src/restore.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
643643
time_t start_time, end_time;
644644

645645
/* Preparations for actual restoring */
646-
time2iso(timestamp, lengthof(timestamp), dest_backup->start_time);
646+
time2iso(timestamp, lengthof(timestamp), dest_backup->start_time, false);
647647
elog(INFO, "Restoring the database from backup at %s", timestamp);
648648

649649
dest_files = get_backup_filelist(dest_backup, true);
@@ -1465,7 +1465,7 @@ pg12_recovery_config(pgBackup *backup, bool add_include)
14651465
{
14661466
char current_time_str[100];
14671467

1468-
time2iso(current_time_str, lengthof(current_time_str), current_time);
1468+
time2iso(current_time_str, lengthof(current_time_str), current_time, false);
14691469

14701470
snprintf(postgres_auto_path, lengthof(postgres_auto_path),
14711471
"%s/postgresql.auto.conf", instance_config.pgdata);

src/show.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -374,12 +374,12 @@ print_backup_json_object(PQExpBuffer buf, pgBackup *backup)
374374
(uint32) (backup->stop_lsn >> 32), (uint32) backup->stop_lsn);
375375
json_add_value(buf, "stop-lsn", lsn, json_level, true);
376376

377-
time2iso(timestamp, lengthof(timestamp), backup->start_time);
377+
time2iso(timestamp, lengthof(timestamp), backup->start_time, false);
378378
json_add_value(buf, "start-time", timestamp, json_level, true);
379379

380380
if (backup->end_time)
381381
{
382-
time2iso(timestamp, lengthof(timestamp), backup->end_time);
382+
time2iso(timestamp, lengthof(timestamp), backup->end_time, false);
383383
json_add_value(buf, "end-time", timestamp, json_level, true);
384384
}
385385

@@ -388,13 +388,13 @@ print_backup_json_object(PQExpBuffer buf, pgBackup *backup)
388388

389389
if (backup->recovery_time > 0)
390390
{
391-
time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
391+
time2iso(timestamp, lengthof(timestamp), backup->recovery_time, false);
392392
json_add_value(buf, "recovery-time", timestamp, json_level, true);
393393
}
394394

395395
if (backup->expire_time > 0)
396396
{
397-
time2iso(timestamp, lengthof(timestamp), backup->expire_time);
397+
time2iso(timestamp, lengthof(timestamp), backup->expire_time, false);
398398
json_add_value(buf, "expire-time", timestamp, json_level, true);
399399
}
400400

@@ -482,7 +482,7 @@ show_backup(const char *instance_name, time_t requested_backup_id)
482482
}
483483

484484
if (show_format == SHOW_PLAIN)
485-
pgBackupWriteControl(stdout, backup);
485+
pgBackupWriteControl(stdout, backup, false);
486486
else
487487
elog(ERROR, "Invalid show format %d", (int) show_format);
488488

@@ -550,7 +550,7 @@ show_instance_plain(const char *instance_name, parray *backup_list, bool show_na
550550
/* Recovery Time */
551551
if (backup->recovery_time != (time_t) 0)
552552
time2iso(row->recovery_time, lengthof(row->recovery_time),
553-
backup->recovery_time);
553+
backup->recovery_time, false);
554554
else
555555
StrNCpy(row->recovery_time, "----", sizeof(row->recovery_time));
556556
widths[cur] = Max(widths[cur], strlen(row->recovery_time));

src/utils/configuration.c

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ option_get_value(ConfigOption *opt)
679679
if (t > 0)
680680
{
681681
timestamp = palloc(100);
682-
time2iso(timestamp, 100, t);
682+
time2iso(timestamp, 100, t, false);
683683
}
684684
else
685685
timestamp = palloc0(1 /* just null termination */);
@@ -1111,6 +1111,8 @@ parse_time(const char *value, time_t *result, bool utc_default)
11111111
struct tm tm;
11121112
char junk[2];
11131113

1114+
char *local_tz = getenv("TZ");
1115+
11141116
/* tmp = replace( value, !isalnum, ' ' ) */
11151117
tmp = pgut_malloc(strlen(value) + + 1);
11161118
len = 0;
@@ -1221,26 +1223,27 @@ parse_time(const char *value, time_t *result, bool utc_default)
12211223
/* determine whether Daylight Saving Time is in effect */
12221224
tm.tm_isdst = -1;
12231225

1226+
/* set timezone to UTC */
1227+
setenv("TZ", "UTC", 1);
1228+
1229+
/* convert time to utc unix time */
12241230
*result = mktime(&tm);
12251231

1232+
/* return old timezone back if any */
1233+
if (local_tz)
1234+
setenv("TZ", local_tz, 1);
1235+
else
1236+
unsetenv("TZ");
1237+
12261238
/* adjust time zone */
12271239
if (tz_set || utc_default)
12281240
{
1229-
time_t ltime = time(NULL);
1230-
struct tm *ptm = gmtime(&ltime);
1231-
time_t gmt = mktime(ptm);
1232-
time_t offset;
1233-
12341241
/* UTC time */
12351242
*result -= tz;
1236-
1237-
/* Get local time */
1238-
ptm = localtime(&ltime);
1239-
offset = ltime - gmt + (ptm->tm_isdst ? 3600 : 0);
1240-
1241-
*result += offset;
12421243
}
12431244

1245+
pg_free(local_tz);
1246+
12441247
return true;
12451248
}
12461249

@@ -1462,14 +1465,32 @@ convert_from_base_unit_u(uint64 base_value, int base_unit,
14621465
* Convert time_t value to ISO-8601 format string. Always set timezone offset.
14631466
*/
14641467
void
1465-
time2iso(char *buf, size_t len, time_t time)
1468+
time2iso(char *buf, size_t len, time_t time, bool utc)
14661469
{
1467-
struct tm *ptm = gmtime(&time);
1468-
time_t gmt = mktime(ptm);
1470+
struct tm *ptm = NULL;
1471+
time_t gmt;
14691472
time_t offset;
14701473
char *ptr = buf;
1474+
char *local_tz = getenv("TZ");
14711475

1476+
/* set timezone to UTC if requested */
1477+
if (utc)
1478+
setenv("TZ", "UTC", 1);
1479+
1480+
ptm = gmtime(&time);
1481+
gmt = mktime(ptm);
14721482
ptm = localtime(&time);
1483+
1484+
if (utc)
1485+
{
1486+
/* return old timezone back if any */
1487+
if (local_tz)
1488+
setenv("TZ", local_tz, 1);
1489+
else
1490+
unsetenv("TZ");
1491+
}
1492+
1493+
/* adjust timezone offset */
14731494
offset = time - gmt + (ptm->tm_isdst ? 3600 : 0);
14741495

14751496
strftime(ptr, len, "%Y-%m-%d %H:%M:%S", ptm);
@@ -1485,4 +1506,6 @@ time2iso(char *buf, size_t len, time_t time)
14851506
snprintf(ptr, len - (ptr - buf), ":%02d",
14861507
abs((int) offset % SECS_PER_HOUR) / SECS_PER_MINUTE);
14871508
}
1509+
1510+
pg_free(local_tz);
14881511
}

src/utils/configuration.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ extern bool parse_int(const char *value, int *result, int flags,
9696
const char **hintmsg);
9797
extern bool parse_lsn(const char *value, XLogRecPtr *result);
9898

99-
extern void time2iso(char *buf, size_t len, time_t time);
99+
extern void time2iso(char *buf, size_t len, time_t time, bool utc);
100100

101101
extern void convert_from_base_unit(int64 base_value, int base_unit,
102102
int64 *value, const char **unit);

0 commit comments

Comments
 (0)