Skip to content

Commit 3ca609a

Browse files
alubennikovaalubennikova
committed
Code cleanup: validate
1 parent 75534a0 commit 3ca609a

File tree

5 files changed

+84
-103
lines changed

5 files changed

+84
-103
lines changed

‎backup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ do_backup(bool smooth_checkpoint)
516516
parray_walk(files_database, pgFileFree);
517517
parray_free(files_database);
518518

519-
pgBackupValidate(&current, false, false);
519+
pgBackupValidate(&current);
520520

521521
return 0;
522522
}

‎pg_probackup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ char arclog_path[MAXPGPATH];
2828
char *backup_id_string_param = NULL;
2929
/* directory configuration */
3030
pgBackup current;
31+
ProbackupSubcmd backup_subcmd;
3132

3233
/* backup configuration */
3334
static bool smooth_checkpoint;
@@ -106,7 +107,6 @@ static pgut_option options[] =
106107
int
107108
main(int argc, char *argv[])
108109
{
109-
ProbackupSubcmd backup_subcmd;
110110
int i;
111111

112112
/* initialize configuration */

‎pg_probackup.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ extern char arclog_path[MAXPGPATH];
224224

225225
/* current settings */
226226
extern pgBackup current;
227+
extern ProbackupSubcmd backup_subcmd;
227228

228229
/* exclude directory list for $PGDATA file listing */
229230
extern const char *pgdata_exclude_dir[];
@@ -286,9 +287,7 @@ extern char *slurpFile(const char *datadir,
286287
bool safe);
287288

288289
/* in validate.c */
289-
extern bool pgBackupValidate(pgBackup *backup,
290-
bool size_only,
291-
bool for_get_timeline);
290+
extern void pgBackupValidate(pgBackup* backup);
292291

293292
extern pgBackup *read_backup(time_t timestamp);
294293
extern void init_backup(pgBackup *backup);

‎restore.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,7 @@ do_restore_or_validate(time_t target_backup_id,
213213

214214
if (backup->status == BACKUP_STATUS_OK);
215215
{
216-
/* TODO fix second argument */
217-
pgBackupValidate(backup, is_restore, false);
216+
pgBackupValidate(backup);
218217

219218
if (is_restore)
220219
restore_database(backup);

‎validate.c

Lines changed: 79 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,15 @@ static void pgBackupValidateFiles(void *arg);
1818
typedef struct
1919
{
2020
parray *files;
21-
const char *root;
22-
bool size_only;
21+
bool validate_crc;
2322
bool corrupted;
2423
} validate_files_args;
2524

2625
/*
27-
* Validate each files in the backup with its size.
28-
* TODO: change from bool to void
29-
* TODO: understand and probably fix second argument
26+
* Validate backup files.
3027
*/
31-
bool
32-
pgBackupValidate(pgBackup *backup,
33-
bool size_only,
34-
bool for_get_timeline)
28+
void
29+
pgBackupValidate(pgBackup *backup)
3530
{
3631
char *backup_id_string;
3732
char base_path[MAXPGPATH];
@@ -40,96 +35,79 @@ pgBackupValidate(pgBackup *backup,
4035
bool corrupted = false;
4136
pthread_t validate_threads[num_threads];
4237
validate_files_args *validate_threads_args[num_threads];
38+
int i;
4339

4440
backup_id_string = base36enc(backup->start_time);
45-
if (!for_get_timeline)
41+
42+
elog(LOG, "Validate backup %s", backup_id_string);
43+
44+
if (backup->backup_mode != BACKUP_MODE_FULL &&
45+
backup->backup_mode != BACKUP_MODE_DIFF_PAGE &&
46+
backup->backup_mode != BACKUP_MODE_DIFF_PTRACK)
47+
elog(LOG, "Invalid backup_mode of backup %s", backup_id_string);
48+
49+
pgBackupGetPath(backup, base_path, lengthof(base_path), DATABASE_DIR);
50+
pgBackupGetPath(backup, path, lengthof(path), DATABASE_FILE_LIST);
51+
files = dir_read_file_list(base_path, path);
52+
53+
/* setup threads */
54+
for (i = 0; i < parray_num(files); i++)
4655
{
47-
if (backup->backup_mode == BACKUP_MODE_FULL ||
48-
backup->backup_mode == BACKUP_MODE_DIFF_PAGE ||
49-
backup->backup_mode == BACKUP_MODE_DIFF_PTRACK)
50-
elog(INFO, "validate: %s backup and archive log files by %s",
51-
backup_id_string, (size_only ? "SIZE" : "CRC"));
56+
pgFile *file = (pgFile *) parray_get(files, i);
57+
__sync_lock_release(&file->lock);
5258
}
5359

54-
if (backup->backup_mode == BACKUP_MODE_FULL ||
55-
backup->backup_mode == BACKUP_MODE_DIFF_PAGE ||
56-
backup->backup_mode == BACKUP_MODE_DIFF_PTRACK)
60+
/* Validate files */
61+
for (i = 0; i < num_threads; i++)
5762
{
58-
int i;
59-
elog(LOG, "database files...");
60-
pgBackupGetPath(backup, base_path, lengthof(base_path), DATABASE_DIR);
61-
pgBackupGetPath(backup, path, lengthof(path),
62-
DATABASE_FILE_LIST);
63-
files = dir_read_file_list(base_path, path);
64-
65-
/* setup threads */
66-
for (i = 0; i < parray_num(files); i++)
67-
{
68-
pgFile *file = (pgFile *) parray_get(files, i);
69-
__sync_lock_release(&file->lock);
70-
}
63+
validate_files_args *arg = pg_malloc(sizeof(validate_files_args));
64+
arg->files = files;
65+
arg->validate_crc = true;
7166

72-
/* restore files into $PGDATA */
73-
for (i = 0; i < num_threads; i++)
74-
{
75-
validate_files_args *arg = pg_malloc(sizeof(validate_files_args));
76-
arg->files = files;
77-
arg->root = base_path;
78-
arg->size_only = size_only;
79-
arg->corrupted = false;
80-
81-
validate_threads_args[i] = arg;
82-
pthread_create(&validate_threads[i], NULL, (void *(*)(void *)) pgBackupValidateFiles, arg);
83-
}
67+
/* TODO Why didn't we validate checksums on restore before? */
68+
// if (backup_subcmd == RESTORE)
69+
// arg->validate_crc = false;
8470

85-
/* Wait theads */
86-
for (i = 0; i < num_threads; i++)
87-
{
88-
pthread_join(validate_threads[i], NULL);
89-
if (validate_threads_args[i]->corrupted)
90-
corrupted = true;
91-
pg_free(validate_threads_args[i]);
92-
}
93-
parray_walk(files, pgFileFree);
94-
parray_free(files);
71+
arg->corrupted = false;
72+
validate_threads_args[i] = arg;
73+
pthread_create(&validate_threads[i], NULL, (void *(*)(void *)) pgBackupValidateFiles, arg);
9574
}
9675

97-
/* update status to OK */
98-
if (corrupted)
99-
backup->status = BACKUP_STATUS_CORRUPT;
100-
else
101-
backup->status = BACKUP_STATUS_OK;
102-
pgBackupWriteConf(backup);
76+
/* Wait theads */
77+
for (i = 0; i < num_threads; i++)
78+
{
79+
pthread_join(validate_threads[i], NULL);
80+
if (validate_threads_args[i]->corrupted)
81+
corrupted = true;
82+
pg_free(validate_threads_args[i]);
83+
}
10384

104-
if (corrupted)
105-
elog(WARNING, "backup %s is corrupted", backup_id_string);
106-
else
107-
elog(LOG, "backup %s is valid", backup_id_string);
85+
/* cleanup */
86+
parray_walk(files, pgFileFree);
87+
parray_free(files);
10888

109-
return !corrupted;
110-
}
89+
/* Update backup status */
90+
backup->status = corrupted ? BACKUP_STATUS_CORRUPT : BACKUP_STATUS_OK;
91+
pgBackupWriteConf(backup);
11192

112-
static const char *
113-
get_relative_path(const char *path, const char *root)
114-
{
115-
size_t rootlen = strlen(root);
116-
if (strncmp(path, root, rootlen) == 0 && path[rootlen] == '/')
117-
return path + rootlen + 1;
93+
if (corrupted)
94+
elog(WARNING, "Backup %s is corrupted", backup_id_string);
11895
else
119-
return path;
96+
elog(LOG, "Backup %s is valid", backup_id_string);
12097
}
12198

12299
/*
123100
* Validate files in the backup with size or CRC.
101+
* NOTE: If file is not valid, do not use ERROR log message,
102+
* rather throw a WARNING and set arguments->corrupted = true.
103+
* This is necessary to update backup status.
124104
*/
125105
static void
126106
pgBackupValidateFiles(void *arg)
127107
{
128108
int i;
129-
130109
validate_files_args *arguments = (validate_files_args *)arg;
131110

132-
133111
for (i = 0; i < parray_num(arguments->files); i++)
134112
{
135113
struct stat st;
@@ -139,51 +117,56 @@ pgBackupValidateFiles(void *arg)
139117
continue;
140118

141119
if (interrupted)
142-
elog(ERROR, "interrupted during validate");
143-
144-
/* skipped backup while differential backup */
145-
/* NOTE We don't compute checksums for compressed data,
146-
* so skip it too */
147-
if (file->write_size == BYTES_INVALID
148-
|| !S_ISREG(file->mode)
149-
|| file->generation != -1)
120+
elog(ERROR, "Interrupted during validate");
121+
122+
/* Validate only regular files */
123+
if (!S_ISREG(file->mode))
124+
continue;
125+
/*
126+
* Skip files which has no data, because they
127+
* haven't changed between backups.
128+
*/
129+
if (file->write_size == BYTES_INVALID)
130+
continue;
131+
/* We don't compute checksums for compressed data, so skip them
132+
* TODO Add checksums to compressed files.
133+
*/
134+
if (file->generation != -1)
150135
continue;
151136

152137
/* print progress */
153-
elog(LOG, "(%d/%lu) %s", i + 1, (unsigned long) parray_num(arguments->files),
154-
get_relative_path(file->path, arguments->root));
138+
elog(LOG, "Validate files: (%d/%lu) %s",
139+
i + 1, (unsigned long) parray_num(arguments->files), file->path);
155140

156-
/* always validate file size */
157141
if (stat(file->path, &st) == -1)
158142
{
159143
if (errno == ENOENT)
160-
elog(WARNING, "backup file \"%s\" vanished", file->path);
144+
elog(WARNING, "Backup file \"%s\" is not found", file->path);
161145
else
162-
elog(ERROR, "cannot stat backup file \"%s\": %s",
163-
get_relative_path(file->path, arguments->root), strerror(errno));
146+
elog(WARNING, "Cannot stat backup file \"%s\": %s",
147+
file->path, strerror(errno));
164148
arguments->corrupted = true;
165149
return;
166150
}
151+
167152
if (file->write_size != st.st_size)
168153
{
169-
elog(WARNING, "size of backup file \"%s\" must be %lu but %lu",
170-
get_relative_path(file->path, arguments->root),
171-
(unsigned long) file->write_size,
172-
(unsigned long) st.st_size);
154+
elog(WARNING, "Invalid size of backup file \"%s\" : %lu. Expected %lu",
155+
file->path, (unsigned long) file->write_size,
156+
(unsigned long) st.st_size);
173157
arguments->corrupted = true;
174158
return;
175159
}
176160

177-
/* validate CRC too */
178-
if (!arguments->size_only)
161+
if (arguments->validate_crc)
179162
{
180163
pg_crc32 crc;
181164

182165
crc = pgFileGetCRC(file);
183166
if (crc != file->crc)
184167
{
185-
elog(WARNING, "CRC of backup file \"%s\" must be %X but %X",
186-
get_relative_path(file->path, arguments->root), file->crc, crc);
168+
elog(WARNING, "Invalid CRC of backup file \"%s\" : %X. Expected %X",
169+
file->path, file->crc, crc);
187170
arguments->corrupted = true;
188171
return;
189172
}

0 commit comments

Comments
 (0)