Skip to content

Commit ddbc6fd

Browse files
committed
[Issue #228] remove compressed_size attribute from BackupPageHeader2
1 parent 76f9267 commit ddbc6fd

File tree

3 files changed

+88
-67
lines changed

3 files changed

+88
-67
lines changed

src/data.c

Lines changed: 74 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ compress_and_backup_page(pgFile *file, BlockNumber blknum,
506506
{
507507
// memcpy(write_buffer, &header, sizeof(header));
508508
memcpy(write_buffer, compressed_page, compressed_size);
509-
write_buffer_size = MAXALIGN(compressed_size);
509+
write_buffer_size = compressed_size;
510510
}
511511
/* Non-positive value means that compression failed. Write it as is. */
512512
else
@@ -875,6 +875,9 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
875875
off_t cur_pos_out = 0;
876876
off_t cur_pos_in = 0;
877877

878+
/* should not be possible */
879+
Assert(!(backup_version >= 20400 && file->n_headers <= 0));
880+
878881
/*
879882
* We rely on stdio buffering of input and output.
880883
* For buffering to be efficient, we try to minimize the
@@ -915,14 +918,23 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
915918
blknum = headers[n_hdr].block;
916919
page_lsn = headers[n_hdr].lsn;
917920
page_crc = headers[n_hdr].checksum;
918-
compressed_size = headers[n_hdr].compressed_size;
921+
/* calculate payload size by comparing current and next page positions */
922+
compressed_size = headers[n_hdr+1].pos - headers[n_hdr].pos;
923+
924+
Assert(compressed_size > 0);
925+
Assert(compressed_size <= BLCKSZ);
926+
927+
read_len = compressed_size;
919928
}
920929
else
921930
{
922931
if (get_compressed_page_meta(in, from_fullpath, &compressed_size,
923932
&blknum, NULL, false))
924933
{
925934
cur_pos_in += sizeof(BackupPageHeader);
935+
936+
/* backward compatibility kludge TODO: remove in 3.0 */
937+
read_len = MAXALIGN(compressed_size);
926938
}
927939
else
928940
break;
@@ -963,6 +975,9 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
963975
break;
964976
}
965977

978+
Assert(compressed_size > 0);
979+
Assert(compressed_size <= BLCKSZ);
980+
966981
/* no point in writing redundant data */
967982
if (nblocks > 0 && blknum >= nblocks)
968983
break;
@@ -992,8 +1007,9 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
9921007
/* if this page is marked as already restored, then skip it */
9931008
if (map && datapagemap_is_set(map, blknum))
9941009
{
995-
// elog(INFO, "HAHAHA");
996-
/* skip to the next page for backup withot header file */
1010+
/* Backward compatibility kludge TODO: remove in 3.0
1011+
* skip to the next page for backup withot header file
1012+
*/
9971013
if (!headers && fseek(in, MAXALIGN(compressed_size), SEEK_CUR) != 0)
9981014
elog(ERROR, "Cannot seek block %u of '%s': %s",
9991015
blknum, from_fullpath, strerror(errno));
@@ -1011,13 +1027,9 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
10111027
}
10121028

10131029
/* read a page from file */
1014-
read_len = fread(page.data, 1, MAXALIGN(compressed_size), in);
1015-
1016-
// elog(INFO, "Blocknum %u, zread: %u ", blknum, compressed_size);
1017-
1018-
if (read_len != MAXALIGN(compressed_size))
1019-
elog(ERROR, "Cannot read block %u of \"%s\", read %zu of %d",
1020-
blknum, from_fullpath, read_len, compressed_size);
1030+
if (fread(page.data, 1, read_len, in) != read_len)
1031+
elog(ERROR, "Cannot read block %u file \"%s\": %s",
1032+
blknum, from_fullpath, strerror(errno));
10211033

10221034
cur_pos_in += read_len;
10231035

@@ -1072,6 +1084,7 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers
10721084
write_len += BLCKSZ;
10731085
cur_pos_out += BLCKSZ; /* update current write position */
10741086

1087+
/* Mark page as restored, to avoid reading this page when restoring parent backups */
10751088
if (map)
10761089
datapagemap_add(map, blknum);
10771090
}
@@ -1553,10 +1566,8 @@ validate_file_pages(pgFile *file, const char *fullpath, XLogRecPtr stop_lsn,
15531566

15541567
elog(VERBOSE, "Validate relation blocks for file \"%s\"", fullpath);
15551568

1556-
/* nothing to validate */
1557-
if (backup_version >= 20400 &&
1558-
file->n_headers <= 0)
1559-
return true;
1569+
/* should not be possible */
1570+
Assert(!(backup_version >= 20400 && file->n_headers <= 0));
15601571

15611572
in = fopen(fullpath, PG_BINARY_R);
15621573
if (in == NULL)
@@ -1592,22 +1603,34 @@ validate_file_pages(pgFile *file, const char *fullpath, XLogRecPtr stop_lsn,
15921603
break;
15931604

15941605
blknum = headers[n_hdr].block;
1595-
compressed_size = headers[n_hdr].compressed_size;
1606+
/* calculate payload size by comparing current and next page positions */
1607+
compressed_size = headers[n_hdr+1].pos - headers[n_hdr].pos;
15961608

1597-
// elog(INFO, "POS: %u", headers[n_hdr].pos);
1609+
Assert(compressed_size > 0);
1610+
Assert(compressed_size <= BLCKSZ);
15981611

1599-
if (cur_pos != headers[n_hdr].pos &&
1600-
fio_fseek(in, headers[n_hdr].pos) < 0)
1612+
if (cur_pos != headers[n_hdr].pos)
16011613
{
1602-
elog(ERROR, "Cannot seek block %u of \"%s\": %s",
1603-
blknum, fullpath, strerror(errno));
1614+
if (fio_fseek(in, headers[n_hdr].pos) < 0)
1615+
elog(ERROR, "Cannot seek block %u of \"%s\": %s",
1616+
blknum, fullpath, strerror(errno));
1617+
1618+
cur_pos = headers[n_hdr].pos;
16041619
}
1620+
1621+
read_len = compressed_size;
16051622
}
1623+
/* old backups rely on header located directly in data file */
16061624
else
16071625
{
16081626
if (!get_compressed_page_meta(in, fullpath, &compressed_size,
16091627
&blknum, &crc, use_crc32c))
16101628
break;
1629+
1630+
/* Backward compatibility kludge, TODO: remove in 3.0
1631+
* for some reason we padded compressed pages in old versions
1632+
*/
1633+
read_len = MAXALIGN(compressed_size);
16111634
}
16121635

16131636
/* backward compatibility kludge TODO: remove in 3.0 */
@@ -1621,21 +1644,16 @@ validate_file_pages(pgFile *file, const char *fullpath, XLogRecPtr stop_lsn,
16211644
Assert(compressed_size <= BLCKSZ);
16221645
Assert(compressed_size > 0);
16231646

1624-
read_len = fread(compressed_page.data, 1, MAXALIGN(compressed_size), in);
1625-
if (read_len != MAXALIGN(compressed_size))
1647+
if (fread(compressed_page.data, 1, read_len, in) != read_len)
16261648
{
1627-
elog(WARNING, "Cannot read block %u of \"%s\" read %zu of %d",
1628-
blknum, fullpath, read_len, compressed_size);
1649+
elog(WARNING, "Cannot read block %u file \"%s\": %s",
1650+
blknum, fullpath, strerror(errno));
16291651
return false;
16301652
}
16311653

1632-
// elog(INFO, "POS1: %lu", cur_pos);
1633-
1654+
/* update current position */
16341655
cur_pos += read_len;
16351656

1636-
// elog(INFO, "POS2: %lu", cur_pos);
1637-
// elog(INFO, "Compressed size: %i", compressed_size);
1638-
16391657
COMP_FILE_CRC32(use_crc32c, crc, compressed_page.data, read_len);
16401658

16411659
if (compressed_size != BLCKSZ
@@ -1895,12 +1913,12 @@ get_compressed_page_meta(FILE *in, const char *fullpath, int32 *compressed_size,
18951913
*blknum = header.block;
18961914
*compressed_size = header.compressed_size;
18971915

1898-
elog(INFO, "blknum: %i", header.block);
1899-
elog(INFO, "size: %i", header.compressed_size);
1900-
elog(INFO, "size2: %i", *compressed_size);
1901-
1902-
elog(INFO, "BLKNUM: %i", *blknum);
1903-
elog(INFO, "File: %s", fullpath);
1916+
// elog(INFO, "blknum: %i", header.block);
1917+
// elog(INFO, "size: %i", header.compressed_size);
1918+
// elog(INFO, "size2: %i", *compressed_size);
1919+
//
1920+
// elog(INFO, "BLKNUM: %i", *blknum);
1921+
// elog(INFO, "File: %s", fullpath);
19041922

19051923
Assert(*compressed_size != 0);
19061924
return true;
@@ -1930,6 +1948,7 @@ open_local_file_rw(const char *to_fullpath, char **out_buf, uint32 buf_size)
19301948
return out;
19311949
}
19321950

1951+
/* backup local file */
19331952
int
19341953
send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_fullpath,
19351954
pgFile *file, XLogRecPtr prev_backup_start_lsn, CompressAlg calg, int clevel,
@@ -1944,6 +1963,7 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f
19441963
int n_blocks_read = 0;
19451964
BlockNumber blknum = 0;
19461965
datapagemap_iterator_t *iter = NULL;
1966+
int compressed_size = 0;
19471967

19481968
/* stdio buffers */
19491969
char *in_buf = NULL;
@@ -2004,17 +2024,17 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f
20042024
if (!*headers)
20052025
*headers = (BackupPageHeader2 *) pgut_malloc(sizeof(BackupPageHeader2));
20062026
else
2007-
*headers = (BackupPageHeader2 *) pgut_realloc(*headers, (hdr_num+1 ) * sizeof(BackupPageHeader2));
2027+
*headers = (BackupPageHeader2 *) pgut_realloc(*headers, (hdr_num+1) * sizeof(BackupPageHeader2));
20082028

20092029
(*headers)[hdr_num].block = blknum;
20102030
(*headers)[hdr_num].pos = cur_pos_out;
20112031
(*headers)[hdr_num].lsn = page_st.lsn;
20122032
(*headers)[hdr_num].checksum = page_st.checksum;
20132033

2014-
(*headers)[hdr_num].compressed_size = compress_and_backup_page(file, blknum, in, out, &(file->crc),
2015-
rc, curr_page, calg, clevel,
2016-
from_fullpath, to_fullpath);
2017-
cur_pos_out += MAXALIGN((*headers)[hdr_num].compressed_size);
2034+
compressed_size = compress_and_backup_page(file, blknum, in, out, &(file->crc),
2035+
rc, curr_page, calg, clevel,
2036+
from_fullpath, to_fullpath);
2037+
cur_pos_out += compressed_size;
20182038
}
20192039

20202040
n_blocks_read++;
@@ -2030,7 +2050,13 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f
20302050
blknum++;
20312051
}
20322052

2033-
file->n_headers = hdr_num +1;
2053+
/* add one additional header */
2054+
if (*headers)
2055+
{
2056+
file->n_headers = hdr_num +1;
2057+
*headers = (BackupPageHeader2 *) pgut_realloc(*headers, (hdr_num+2) * sizeof(BackupPageHeader2));
2058+
(*headers)[hdr_num+1].pos = cur_pos_out;
2059+
}
20342060

20352061
/* cleanup */
20362062
if (in && fclose(in))
@@ -2061,19 +2087,11 @@ get_data_file_headers(const char *fullpath, pgFile *file, uint32 backup_version)
20612087
char fullpath_hdr[MAXPGPATH];
20622088
BackupPageHeader2 *headers = NULL;
20632089

2064-
// elog(INFO, "Backup Version: %u", backup_version);
2065-
20662090
if (backup_version < 20400)
2067-
{
2068-
elog(INFO, "HELLO1");
20692091
return NULL;
2070-
}
20712092

20722093
if (file->n_headers <= 0)
2073-
{
2074-
elog(INFO, "HELLO2");
20752094
return NULL;
2076-
}
20772095

20782096
snprintf(fullpath_hdr, MAXPGPATH, "%s_hdr", fullpath);
20792097

@@ -2082,7 +2100,11 @@ get_data_file_headers(const char *fullpath, pgFile *file, uint32 backup_version)
20822100
if (!in)
20832101
elog(ERROR, "Cannot open header file \"%s\": %s", fullpath_hdr, strerror(errno));
20842102

2085-
len = file->n_headers * sizeof(BackupPageHeader2);
2103+
/*
2104+
* the actual number of headers in header file is n+1, last one is a dummy header,
2105+
* used for calculation of compressed_size for actual last header.
2106+
*/
2107+
len = (file->n_headers+1) * sizeof(BackupPageHeader2);
20862108
headers = pgut_malloc(len);
20872109

20882110
if (fread(headers, 1, len, in) != len)
@@ -2094,10 +2116,8 @@ get_data_file_headers(const char *fullpath, pgFile *file, uint32 backup_version)
20942116
FIN_FILE_CRC32(true, hdr_crc);
20952117

20962118
if (hdr_crc != file->hdr_crc)
2097-
{
20982119
elog(ERROR, "Header file crc mismatch \"%s\", current: %u, expected: %u",
20992120
fullpath_hdr, hdr_crc, file->hdr_crc);
2100-
}
21012121

21022122
if (fclose(in))
21032123
elog(ERROR, "Cannot close header file \"%s\": %s", fullpath_hdr, strerror(errno));
@@ -2127,7 +2147,7 @@ write_page_headers(BackupPageHeader2 *headers, pgFile *file, const char* to_full
21272147
elog(ERROR, "Cannot change mode of \"%s\": %s", to_fullpath,
21282148
strerror(errno));
21292149

2130-
hdr_size = file->n_headers * sizeof(BackupPageHeader2);
2150+
hdr_size = (file->n_headers+1) * sizeof(BackupPageHeader2);
21312151

21322152
/* calculate checksums */
21332153
INIT_FILE_CRC32(true, file->hdr_crc);

src/pg_probackup.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,10 +585,9 @@ typedef struct BackupPageHeader
585585
/* 4MB for 1GB file */
586586
typedef struct BackupPageHeader2
587587
{
588+
XLogRecPtr lsn;
588589
int32 block; /* block number */
589590
int32 pos; /* position in backup file */
590-
int32 compressed_size;
591-
XLogRecPtr lsn;
592591
uint16 checksum;
593592
} BackupPageHeader2;
594593

src/utils/file.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,9 +1427,6 @@ int fio_send_pages(const char *to_fullpath, const char *from_fullpath, pgFile *f
14271427
--------------------------------------------------------------
14281428
*/
14291429

1430-
// elog(WARNING, "Size: %lu", sizeof(fio_header));
1431-
// elog(ERROR, "Size: %lu", MAXALIGN(sizeof(fio_header)));
1432-
14331430
req.hdr.cop = FIO_SEND_PAGES;
14341431

14351432
if (pagemap)
@@ -1520,7 +1517,7 @@ int fio_send_pages(const char *to_fullpath, const char *from_fullpath, pgFile *f
15201517
{
15211518
*headers = pgut_malloc(hdr.size);
15221519
IO_CHECK(fio_read_all(fio_stdin, *headers, hdr.size), hdr.size);
1523-
file->n_headers = hdr.size / sizeof(BackupPageHeader2);
1520+
file->n_headers = (hdr.size / sizeof(BackupPageHeader2)) -1;
15241521
}
15251522

15261523
break;
@@ -1590,7 +1587,7 @@ static void fio_send_pages_impl(int out, char* buf)
15901587
datapagemap_iterator_t *iter = NULL;
15911588
/* page headers */
15921589
int32 hdr_num = -1;
1593-
int32 hdr_cur_pos = 0;
1590+
int32 cur_pos_out = 0;
15941591
BackupPageHeader2 *headers = NULL;
15951592

15961593
/* open source file */
@@ -1767,7 +1764,7 @@ static void fio_send_pages_impl(int out, char* buf)
17671764
memcpy(write_buffer, read_buffer, BLCKSZ);
17681765
compressed_size = BLCKSZ;
17691766
}
1770-
hdr.size = MAXALIGN(compressed_size);
1767+
hdr.size = compressed_size;
17711768

17721769
IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr));
17731770
IO_CHECK(fio_write_all(out, write_buffer, hdr.size), hdr.size);
@@ -1777,15 +1774,14 @@ static void fio_send_pages_impl(int out, char* buf)
17771774
if (!headers)
17781775
headers = (BackupPageHeader2 *) pgut_malloc(sizeof(BackupPageHeader2));
17791776
else
1780-
headers = (BackupPageHeader2 *) pgut_realloc(headers, (hdr_num+1 ) * sizeof(BackupPageHeader2));
1777+
headers = (BackupPageHeader2 *) pgut_realloc(headers, (hdr_num+1) * sizeof(BackupPageHeader2));
17811778

17821779
headers[hdr_num].block = blknum;
17831780
headers[hdr_num].lsn = page_st.lsn;
17841781
headers[hdr_num].checksum = page_st.checksum;
1785-
headers[hdr_num].pos = hdr_cur_pos;
1786-
headers[hdr_num].compressed_size = compressed_size;
1782+
headers[hdr_num].pos = cur_pos_out;
17871783

1788-
hdr_cur_pos += hdr.size;
1784+
cur_pos_out += compressed_size;
17891785
}
17901786

17911787
/* next block */
@@ -1806,7 +1802,13 @@ static void fio_send_pages_impl(int out, char* buf)
18061802
hdr.size = 0;
18071803

18081804
if (headers)
1809-
hdr.size = (hdr_num+1) * sizeof(BackupPageHeader2);
1805+
{
1806+
hdr.size = (hdr_num+2) * sizeof(BackupPageHeader2);
1807+
1808+
/* add one additional header */
1809+
headers = (BackupPageHeader2 *) pgut_realloc(headers, (hdr_num+2) * sizeof(BackupPageHeader2));
1810+
headers[hdr_num+1].pos = cur_pos_out;
1811+
}
18101812
IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr));
18111813

18121814
if (headers)

0 commit comments

Comments
 (0)