Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite8b9ca1

Browse files
committed
code cleanup, add comments
1 parent52bf25c commite8b9ca1

File tree

6 files changed

+65
-51
lines changed

6 files changed

+65
-51
lines changed

‎src/backup.c

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,10 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo)
315315
dir_list_file(backup_files_list,instance_config.pgdata,
316316
true, true, false,0,FIO_DB_HOST);
317317

318-
/* create database_map used for partial restore */
318+
/*
319+
* Get database_map (name to oid) for use in partial restore feature.
320+
* It's possible that we fail and database_map will be NULL.
321+
*/
319322
database_map=get_database_map(pg_startbackup_conn);
320323

321324
/*
@@ -573,7 +576,7 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo)
573576
if (database_map)
574577
{
575578
write_database_map(&current,database_map,backup_files_list);
576-
/*we don`t need it anymore */
579+
/*cleanup */
577580
parray_walk(database_map,db_map_entry_free);
578581
parray_free(database_map);
579582
}
@@ -1065,8 +1068,15 @@ pg_ptrack_support(PGconn *backup_conn)
10651068
}
10661069

10671070
/*
1068-
* Create 'datname to Oid' map
1069-
* Return NULL if failed to construct database_map // TODO doesn't look safe. See comment below.
1071+
* Fill 'datname to Oid' map
1072+
*
1073+
* This function can fail to get the map for legal reasons, e.g. missing
1074+
* permissions on pg_database during `backup`.
1075+
* As long as user do not use partial restore feature it`s fine.
1076+
*
1077+
* To avoid breaking a backward compatibility don't throw an ERROR,
1078+
* throw a warning instead of an error and return NULL.
1079+
* Caller is responsible for checking the result.
10701080
*/
10711081
parray*
10721082
get_database_map(PGconn*conn)
@@ -1075,18 +1085,16 @@ get_database_map(PGconn *conn)
10751085
parray*database_map=NULL;
10761086
inti;
10771087

1078-
/* TODO add a comment why we exclude template0 and template1 from the map */
1088+
/*
1089+
* Do not include template0 and template1 to the map
1090+
* as default databases that must always be restored.
1091+
*/
10791092
res=pgut_execute_extended(conn,
10801093
"SELECT oid, datname FROM pg_catalog.pg_database "
10811094
"WHERE datname NOT IN ('template1', 'template0')",
10821095
0,NULL, true, true);
10831096

1084-
1085-
/* TODO How is that possible? Shouldn't instance have at least one database?
1086-
* How can we distinguish case when instance only has template databases
1087-
* and case of query failure?
1088-
* Is it ok to ignore the failure?
1089-
*/
1097+
/* Don't error out, simply return NULL. See comment above. */
10901098
if (PQresultStatus(res)!=PGRES_TUPLES_OK)
10911099
{
10921100
PQclear(res);
@@ -1116,15 +1124,6 @@ get_database_map(PGconn *conn)
11161124
parray_append(database_map,db_entry);
11171125
}
11181126

1119-
/* extra paranoia */
1120-
// TODO This code block has no value. Let's delete it.
1121-
if (database_map&& (parray_num(database_map)==0))
1122-
{
1123-
parray_free(database_map);
1124-
elog(WARNING,"Failed to get database map");
1125-
returnNULL;
1126-
}
1127-
11281127
returndatabase_map;
11291128
}
11301129

‎src/data.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ create_empty_file(fio_location from_location, const char *to_root,
10961096
}
10971097

10981098
if (fio_fclose(out))
1099-
elog(ERROR,"cannotwrite \"%s\": %s",to_path,strerror(errno));
1099+
elog(ERROR,"cannotclose \"%s\": %s",to_path,strerror(errno));
11001100

11011101
return true;
11021102
}

‎src/dir.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,22 @@ BlackListCompare(const void *str1, const void *str2)
445445
returnstrcmp(*(char**)str1,*(char**)str2);
446446
}
447447

448+
/* Compare two Oids */
449+
int
450+
pgCompareOid(constvoid*f1,constvoid*f2)
451+
{
452+
Oidv1=*(Oid*)f1;
453+
Oidv2=*(Oid*)f2;
454+
455+
elog(WARNING,"pgCompareOid %u %u",v1,v2);
456+
if (v1>v2)
457+
return1;
458+
elseif (v1<v2)
459+
return-1;
460+
else
461+
return0;}
462+
463+
448464
void
449465
db_map_entry_free(void*entry)
450466
{
@@ -1679,7 +1695,7 @@ print_database_map(FILE *out, parray *database_map)
16791695
}
16801696

16811697
/*
1682-
* Create file 'database_map' and add its meta tobackup_content.control
1698+
* Create file 'database_map' and add its meta tobackup_files_list
16831699
* NULL check for database_map must be done by the caller.
16841700
*/
16851701
void
@@ -1709,8 +1725,10 @@ write_database_map(pgBackup *backup, parray *database_map, parray *backup_files_
17091725
/* Add metadata to backup_content.control */
17101726
file=pgFileNew(database_map_path,DATABASE_MAP, true,0,
17111727
FIO_BACKUP_HOST);
1712-
file->crc=pgFileGetCRC(file->path, true, false,
1713-
&file->read_size,FIO_BACKUP_HOST);
1728+
pfree(file->path);
1729+
file->path=strdup(DATABASE_MAP);
1730+
file->crc=pgFileGetCRC(database_map_path, true, false,
1731+
&file->read_size,FIO_BACKUP_HOST);
17141732
file->write_size=file->read_size;
17151733
parray_append(backup_files_list,file);
17161734
}

‎src/pg_probackup.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,6 @@ main(int argc, char *argv[])
641641
if (datname_exclude_list&&datname_include_list)
642642
elog(ERROR,"You cannot specify '--db-include' and '--db-exclude' together");
643643

644-
/* At this point we are sure that user requested partial restore */
645644
if (datname_exclude_list)
646645
datname_list=datname_exclude_list;
647646

@@ -777,7 +776,7 @@ opt_datname_exclude_list(ConfigOption *opt, const char *arg)
777776

778777
dbname=pgut_malloc(strlen(arg)+1);
779778

780-
/* add sanity for database name */
779+
/*TODOadd sanity for database name */
781780
strcpy(dbname,arg);
782781

783782
parray_append(datname_exclude_list,dbname);
@@ -794,7 +793,7 @@ opt_datname_include_list(ConfigOption *opt, const char *arg)
794793

795794
dbname=pgut_malloc(strlen(arg)+1);
796795

797-
/* add sanity for database name */
796+
/*TODOadd sanity for database name */
798797
strcpy(dbname,arg);
799798

800799
parray_append(datname_include_list,dbname);

‎src/pg_probackup.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,7 @@ extern int pgFileComparePathDesc(const void *f1, const void *f2);
654654
externintpgFileComparePathWithExternalDesc(constvoid*f1,constvoid*f2);
655655
externintpgFileCompareLinked(constvoid*f1,constvoid*f2);
656656
externintpgFileCompareSize(constvoid*f1,constvoid*f2);
657+
externintpgCompareOid(constvoid*f1,constvoid*f2);
657658

658659
/* in data.c */
659660
externboolcheck_data_file(ConnectionArgs*arguments,pgFile*file,uint32checksum_version);

‎src/restore.c

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ static void *restore_files(void *arg);
4444

4545
staticparray*get_dbOid_exclude_list(pgBackup*backup,parray*files,
4646
parray*datname_list,boolpartial_restore_type);
47-
48-
staticintpgCompareOid(constvoid*f1,constvoid*f2);
4947
staticvoidset_orphan_status(parray*backups,pgBackup*parent_backup);
5048

5149
/*
@@ -482,7 +480,7 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt,
482480
}
483481

484482
/*
485-
*At least restore backups files starting from the parent backup.
483+
*Restore backups files starting from the parent backup.
486484
*/
487485
for (i=parray_num(parent_chain)-1;i >=0;i--)
488486
{
@@ -1185,27 +1183,38 @@ get_dbOid_exclude_list(pgBackup *backup, parray *files,
11851183
intj;
11861184
parray*database_map=NULL;
11871185
parray*dbOid_exclude_list=NULL;
1188-
boolfound_database_map= false;
1186+
pgFile*database_map_file=NULL;
1187+
pg_crc32crc;
1188+
charpath[MAXPGPATH];
1189+
chardatabase_map_path[MAXPGPATH];
11891190

11901191
/* make sure that database_map is in backup_content.control */
1191-
// TODO can't we use parray_bsearch here?
11921192
for (i=0;i<parray_num(files);i++)
11931193
{
11941194
pgFile*file= (pgFile*)parray_get(files,i);
11951195

11961196
if ((file->external_dir_num==0)&&
1197-
strcmp(DATABASE_MAP,file->rel_path)==0)
1197+
strcmp(DATABASE_MAP,file->name)==0)
11981198
{
1199-
found_database_map=true;
1199+
database_map_file=file;
12001200
break;
12011201
}
12021202
}
12031203

1204-
// TODO rephrase error message
1205-
if (!found_database_map)
1206-
elog(ERROR,"Backup %s has missing database_map, partial restore is impossible.",
1204+
if (!database_map_file)
1205+
elog(ERROR,"Backup %s doesn't contain a database_map, partial restore is impossible.",
12071206
base36enc(backup->start_time));
12081207

1208+
pgBackupGetPath(backup,path,lengthof(path),DATABASE_DIR);
1209+
join_path_components(database_map_path,path,DATABASE_MAP);
1210+
1211+
/* check database_map CRC */
1212+
crc=pgFileGetCRC(database_map_path, true, true,NULL,FIO_LOCAL_HOST);
1213+
1214+
if (crc!=database_map_file->crc)
1215+
elog(ERROR,"Invalid CRC of backup file \"%s\" : %X. Expected %X",
1216+
database_map_file->path,crc,database_map_file->crc);
1217+
12091218
/* get database_map from file */
12101219
database_map=read_database_map(backup);
12111220

@@ -1215,12 +1224,12 @@ get_dbOid_exclude_list(pgBackup *backup, parray *files,
12151224
base36enc(backup->start_time));
12161225

12171226
/*
1218-
* So we have a list of datnames and database_map for it.
1227+
* So we have a list of datnames andadatabase_map for it.
12191228
* We must construct a list of dbOids to exclude.
12201229
*/
12211230
if (partial_restore_type)
12221231
{
1223-
/* For 'include'find dbOid of every datname NOT specified by user */
1232+
/* For 'include'keep dbOid of every datname NOT specified by user */
12241233
for (i=0;i<parray_num(datname_list);i++)
12251234
{
12261235
boolfound_match= false;
@@ -1294,15 +1303,3 @@ get_dbOid_exclude_list(pgBackup *backup, parray *files,
12941303

12951304
returndbOid_exclude_list;
12961305
}
1297-
1298-
/* Compare two Oids */
1299-
// TODO is it overflow safe?
1300-
// TODO move it to dir.c like other compare functions
1301-
int
1302-
pgCompareOid(constvoid*f1,constvoid*f2)
1303-
{
1304-
Oid*f1p=*(Oid**)f1;
1305-
Oid*f2p=*(Oid**)f2;
1306-
1307-
return (*(Oid*)f1p-*(Oid*)f2p);
1308-
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp