Hello,
>Yes, that's caused by ccb161b. Attached are rebased versions.
Following are some comments,
>uint16 hole_offset:15, /* number of bytes in "hole" */
Typo in description of hole_offset
> for (block_id = 0; block_id <= record->max_block_id; block_id++)
>- {
>- if (XLogRecHasBlockImage(record, block_id))
>- fpi_len += BLCKSZ -
record->blocks[block_id].hole_length;
>- }
>+ fpi_len += record->blocks[block_id].bkp_len;
IIUC, if condition, /if(XLogRecHasBlockImage(record, block_id))/ is
incorrectly removed from the above for loop.
>typedef struct XLogRecordCompressedBlockImageHeader
I am trying to understand the purpose behind declaration of the above
struct. IIUC, it is defined in order to introduce new field uint16
raw_length and it has been declared as a separate struct from
XLogRecordBlockImageHeader to not affect the size of WAL record when
compression is off.
I wonder if it is ok to simply memcpy the uint16 raw_length in the
hdr_scratch when compression is on
and not have a separate header struct for it neither declare it in existing
header. raw_length can be a locally defined variable is XLogRecordAssemble
or it can be a field in registered_buffer struct like compressed_page.
I think this can simplify the code.
Am I missing something obvious?
> /*
> * Fill in the remaining fields in the XLogRecordBlockImageHeader
> * struct and add new entries in the record chain.
> */
> bkpb.fork_flags |= BKPBLOCK_HAS_IMAGE;
This code line seems to be misplaced with respect to the above comment.
Comment indicates filling of XLogRecordBlockImageHeader fields while
fork_flags is a field of XLogRecordBlockHeader.
Is it better to place the code close to following condition?
if (needs_backup)
{
>+ *the original length of the
>+ * block without its page hole being deducible from the compressed data
>+ * itself.
IIUC, this comment before XLogRecordBlockImageHeader seems to be no longer
valid as original length is not deducible from compressed data and rather
stored in header.
Thank you,
Rahila Syed
--
View this message in context:
http://postgresql.nabble.com/Compression-of-full-page-writes-tp5769039p5833025.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
