While reading patch-3 (3-allow-wal-record-header-to-be-split.patch) of
WAL Format
Changes(http://archives.postgresql.org/message-id/4fda5136.6080...@enterprisedb.com),
I had few observations which are summarized below:
1.
ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
+ /*
+ * If we got the whole header already, validate it immediately. Otherwise
+ * we validate it after reading the rest of the header from the next page.
+ */
+ if (targetRecOff <= XLOG_BLCKSZ - SizeOfXLogRecord)
+ {
+ if (!ValidXLogRecordHeader(RecPtr, record, emode, randAccess))
+ goto next_record_is_invalid;
+ gotheader = true;
+ }
+ else
+ gotheader = false;
+
Shouldn't the record header validation be done before the check for allocating
a bigger record buffer based
on total length. Otherwise it may lead to allocation of bigger buffer which may
not be required if record header is invalid.
In cases where record header is not split, validation can be done before
otherwise it can be done later.
3. General observation, not related to your changes
XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata)
.
.
if (freespace == 0)
{
updrqst = AdvanceXLInsertBuffer(false);
In the code, AdvanceXLInsertBuffer(false); is called after starting critical
section and acquiring
WALInsertLock, now if any error occurs inside AdvanceXLInsertBuffer()
(in one of the paths it calls XLogWrite(), from which it can call
XLogFileInit() where error can occur);
how will it release WALInsertLock or end critical section.
With Regards,
Amit Kapila.
