While reading patch-3 (3-allow-wal-record-header-to-be-split.patch) of
WAL Format
Changes(http://archives.postgresql.org/message-id/4FDA5136.6080206@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.
