Skip to content

Commit 57d1fa7

Browse files
alubennikovaalubennikova
committed
comments cleanup
1 parent 310882b commit 57d1fa7

File tree

8 files changed

+37
-50
lines changed

8 files changed

+37
-50
lines changed

‎archive.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* --wal-file-path %p --wal-file-name %f', to move backups into arclog_path.
1919
* Where archlog_path is $BACKUP_PATH/wal/system_id.
2020
* Currently it just copies wal files to the new location.
21-
* TODO Planned options: compress, list the arclog content,
21+
* TODO: Planned options: compress, list the arclog content,
2222
* compute and validate checksums.
2323
*/
2424
int

‎backup.c

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ pg_switch_wal(void)
654654

655655
/*
656656
* Check if the instance supports ptrack
657-
* TODO Implement check of ptrack_version() instead of existing one
657+
* TODO Maybe we should rather check ptrack_version()?
658658
*/
659659
static bool
660660
pg_ptrack_support(void)
@@ -959,7 +959,6 @@ pg_stop_backup(pgBackup *backup)
959959
fwrite(PQgetvalue(res, 0, 1), 1, strlen(PQgetvalue(res, 0, 1)), fp);
960960
fclose(fp);
961961

962-
/* TODO What for do we save the file into backup_list? */
963962
/*
964963
* It's vital to check if backup_files_list is initialized,
965964
* because we could get here because the backup was interrupted
@@ -1213,7 +1212,6 @@ backup_compressed_file_partially(pgFile *file, void *arg, size_t *skip_size)
12131212
* verify checksum and copy.
12141213
* In incremental backup mode, copy only files or datafiles' pages changed after
12151214
* previous backup.
1216-
* TODO review
12171215
*/
12181216
static void
12191217
backup_files(void *arg)
@@ -1268,31 +1266,11 @@ backup_files(void *arg)
12681266

12691267
if (S_ISREG(buf.st_mode))
12701268
{
1271-
/* skip files which have not been modified since last backup */
1272-
/* TODO Implement: compare oldfile and newfile checksum. Now it's just a stub */
1273-
if (arguments->prev_backup_filelist)
1274-
{
1275-
pgFile *prev_file = NULL;
1276-
pgFile **p = (pgFile **) parray_bsearch(arguments->prev_backup_filelist,
1277-
file, pgFileComparePath);
1278-
if (p)
1279-
prev_file = *p;
1280-
1281-
if (prev_file && false)
1282-
{
1283-
file->write_size = BYTES_INVALID;
1284-
elog(LOG, "File \"%s\" has not changed since previous backup",
1285-
file->path);
1286-
continue;
1287-
}
1288-
}
1289-
12901269
/* copy the file into backup */
12911270
if (file->is_datafile)
12921271
{
12931272
if (is_compressed_data_file(file))
12941273
{
1295-
/* TODO review */
12961274
size_t skip_size = 0;
12971275
if (backup_compressed_file_partially(file, arguments, &skip_size))
12981276
{
@@ -1345,7 +1323,6 @@ backup_files(void *arg)
13451323

13461324
/*
13471325
* Append files to the backup list array.
1348-
* TODO review
13491326
*/
13501327
static void
13511328
add_pgdata_files(parray *files, const char *root)
@@ -1370,7 +1347,7 @@ add_pgdata_files(parray *files, const char *root)
13701347
/* data files are under "base", "global", or "pg_tblspc" */
13711348
relative = GetRelativePath(file->path, root);
13721349
if (!path_is_prefix_of_path("base", relative) &&
1373-
/*!path_is_prefix_of_path("global", relative) &&*/ //TODO What's wrong with this line?
1350+
!path_is_prefix_of_path("global", relative) &&
13741351
!path_is_prefix_of_path(PG_TBLSPC_DIR, relative))
13751352
continue;
13761353

@@ -1600,7 +1577,10 @@ process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno)
16001577
pg_free(rel_path);
16011578
}
16021579

1603-
/* TODO review it */
1580+
/*
1581+
* Given a list of files in the instance to backup, build a pagemap for each
1582+
* data file that has ptrack. Result is saved in the pagemap field of pgFile.
1583+
*/
16041584
static void
16051585
make_pagemap_from_ptrack(parray *files)
16061586
{
@@ -1622,6 +1602,8 @@ make_pagemap_from_ptrack(parray *files)
16221602
size_t ptrack_nonparsed_size = 0;
16231603
size_t start_addr;
16241604

1605+
/* Compute db_oid and rel_oid of the relation from the path */
1606+
16251607
tablespace = strstr(p->ptrack_path, PG_TBLSPC_DIR);
16261608

16271609
if (tablespace)
@@ -1647,19 +1629,25 @@ make_pagemap_from_ptrack(parray *files)
16471629
p->path);
16481630

16491631
sscanf(p->path + sep_iter + 1, "%u/%u", &db_oid, &rel_oid);
1650-
1632+
1633+
/* get ptrack map for all segments of the relation in a raw format */
16511634
ptrack_nonparsed = pg_ptrack_get_and_clear(tablespace_oid, db_oid,
16521635
rel_oid, &ptrack_nonparsed_size);
16531636

1654-
/* TODO What is 8? */
1655-
start_addr = (RELSEG_SIZE/8)*p->segno;
1656-
if (start_addr + RELSEG_SIZE/8 > ptrack_nonparsed_size)
1637+
/*
1638+
* FIXME When do we cut VARHDR from ptrack_nonparsed?
1639+
* Compute the beginning of the ptrack map related to this segment
1640+
*/
1641+
start_addr = (RELSEG_SIZE/HEAPBLOCKS_PER_BYTE)*p->segno;
1642+
1643+
if (start_addr + RELSEG_SIZE/HEAPBLOCKS_PER_BYTE > ptrack_nonparsed_size)
16571644
p->pagemap.bitmapsize = ptrack_nonparsed_size - start_addr;
16581645
else
1659-
p->pagemap.bitmapsize = RELSEG_SIZE/8;
1646+
p->pagemap.bitmapsize = RELSEG_SIZE/HEAPBLOCKS_PER_BYTE;
16601647

16611648
p->pagemap.bitmap = pg_malloc(p->pagemap.bitmapsize);
16621649
memcpy(p->pagemap.bitmap, ptrack_nonparsed+start_addr, p->pagemap.bitmapsize);
1650+
16631651
pg_free(ptrack_nonparsed);
16641652
}
16651653
}
@@ -1815,7 +1803,7 @@ StreamLog(void *arg)
18151803
* cfs_mmap() and cfs_munmap() function definitions mirror ones
18161804
* from cfs.h, but doesn't use atomic variables, since they are
18171805
* not allowed in frontend code.
1818-
* TODO Is it so?
1806+
*
18191807
* Since we cannot take atomic lock on files compressed by CFS,
18201808
* it should care about not changing files while backup is running.
18211809
*/

‎data.c

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,6 @@ backup_data_page(pgFile *file, XLogRecPtr prev_backup_start_lsn,
147147
* If after several attempts page header is still invalid, throw an error.
148148
* The same idea is applied to checksum verification.
149149
*/
150-
151-
/*
152-
* TODO Should we show a hint about possible false positives suggesting to
153-
* decrease concurrent load? Or we can just copy this page and rely on
154-
* xlog recovery, marking backup as untrusted.
155-
*/
156150
if (!parse_page(&page, &page_lsn))
157151
{
158152
int i;
@@ -346,6 +340,7 @@ backup_data_file(const char *from_root, const char *to_root,
346340
n_blocks_read++;
347341
}
348342

343+
pg_free(&file->pagemap);
349344
pg_free(iter);
350345
}
351346

@@ -364,7 +359,7 @@ backup_data_file(const char *from_root, const char *to_root,
364359

365360
FIN_CRC32C(file->crc);
366361

367-
/* Treat empty file as not-datafile. TODO Why? */
362+
/* Treat empty file as not-datafile. */
368363
if (file->read_size == 0)
369364
file->is_datafile = false;
370365

@@ -385,7 +380,6 @@ backup_data_file(const char *from_root, const char *to_root,
385380

386381
/*
387382
* Restore compressed file that was backed up partly.
388-
* TODO review
389383
*/
390384
static void
391385
restore_file_partly(const char *from_root,const char *to_root, pgFile *file)
@@ -563,8 +557,7 @@ restore_data_file(const char *from_root,
563557
if (header.block < blknum)
564558
elog(ERROR, "backup is broken at block %u", blknum);
565559

566-
/* TODO fix this assert */
567-
Assert (header.compressed_size <= BLCKSZ);
560+
Assert(header.compressed_size <= BLCKSZ);
568561

569562
read_len = fread(compressed_page.data, 1,
570563
MAXALIGN(header.compressed_size), in);

‎dir.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ dir_list_file(parray *files, const char *root, bool exclude, bool omit_symlink,
345345
}
346346

347347
/*
348-
* TODO Add comment, review
348+
* TODO Add comment
349349
*/
350350
static void
351351
dir_list_file_internal(parray *files, const char *root, bool exclude,
@@ -679,7 +679,6 @@ print_file_list(FILE *out, const parray *files, const char *root)
679679
if (file->is_datafile)
680680
fprintf(out, ",\"segno\":\"%d\"", file->segno);
681681

682-
/* TODO What for do we write it to file? */
683682
if (S_ISLNK(file->mode))
684683
fprintf(out, ",\"linked\":\"%s\"", file->linked);
685684

‎pg_probackup.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333

3434
#include "datapagemap.h"
3535

36+
/*
37+
* Macro needed to parse ptrack.
38+
* NOTE Keep those values syncronised with definitions in ptrack.h
39+
*/
40+
#define PTRACK_BITS_PER_HEAPBLOCK 1
41+
#define HEAPBLOCKS_PER_BYTE (BITS_PER_BYTE / PTRACK_BITS_PER_HEAPBLOCK)
3642

3743
/* Directory/File names */
3844
#define DATABASE_DIR "database"

‎restore.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ do_restore_or_validate(time_t target_backup_id,
219219

220220
/*
221221
* Validate corresponding WAL files.
222-
* TODO Shouldn't we pass recovery_target_timeline as last argument?
222+
* We pass base_full_backup timeline as last argument to this function,
223+
* because it's needed to form the name of xlog file.
223224
*/
224225
validate_wal(dest_backup, arclog_path, rt->recovery_target_time,
225226
rt->recovery_target_xid, base_full_backup->tli);
@@ -246,7 +247,7 @@ do_restore_or_validate(time_t target_backup_id,
246247
if (dest_backup->backup_mode != BACKUP_MODE_FULL)
247248
remove_deleted_files(dest_backup);
248249

249-
/* TODO Add comment */
250+
/* Create recovery.conf with given recovery target parameters */
250251
if (!dest_backup->stream
251252
|| (target_time != NULL || target_xid != NULL))
252253
{
@@ -867,7 +868,6 @@ satisfy_timeline(const parray *timelines, const pgBackup *backup)
867868
/*
868869
* Get recovery options in the string format, parse them
869870
* and fill up the pgRecoveryTarget structure.
870-
* TODO move arguments parsing and validation to getopt.
871871
*/
872872
pgRecoveryTarget *
873873
parseRecoveryTargetOptions(const char *target_time,

‎show.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ pretty_size(int64 size, char *buf, size_t len)
162162
}
163163
}
164164

165-
/* TODO Add comment */
166165
static TimeLineID
167166
get_parent_tli(TimeLineID child_tli)
168167
{

‎util.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size)
7575
checkControlFile(ControlFile);
7676
}
7777

78-
/* TODO Add comment */
78+
/*
79+
* Get lsn of the moment when ptrack was enabled the last time.
80+
*/
7981
XLogRecPtr
8082
get_last_ptrack_lsn(void)
8183
{

0 commit comments

Comments
 (0)