On 2021/07/19 18:52, Yugo NAGATA wrote:
Well, I think my first patch was not wrong. The difference with the latest
patch is just whether we perform the additional check when we are not in
standby mode or not. The behavior is basically the same although which function
detects and prints the page-header error in cases of crash recovery is
different.
Yes, so which patch do you think is better? I like your version
because there seems no reason why XLogPageRead() should skip
XLogReaderValidatePageHeader() when not in standby mode.
Also I'm tempted to move ereport() and reset of errmsg_buf to
under next_record_is_invalid as follows. That is, in standby mode
whenever we find an invalid record and retry reading WAL page
in XLogPageRead(), we report the error message and reset it.
For now in XLogPageRead(), only XLogReaderValidatePageHeader()
sets errmsg_buf, but in the future other code or function doing that
may be added. For that case, the following change seems more elegant.
Thought?
* shouldn't be a big deal from a performance point of view.
*/
if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
- {
- /* reset any error XLogReaderValidatePageHeader() might have
set */
- xlogreader->errormsg_buf[0] = '\0';
goto next_record_is_invalid;
- }
return readLen;
@@ -12517,7 +12513,17 @@ next_record_is_invalid:
/* In standby-mode, keep trying */
if (StandbyMode)
+ {
+ if (xlogreader->errormsg_buf[0])
+ {
+ ereport(emode_for_corrupt_record(emode, EndRecPtr),
+ (errmsg_internal("%s",
xlogreader->errormsg_buf)));
+
+ /* reset any error XLogReaderValidatePageHeader() might
have set */
+ xlogreader->errormsg_buf[0] = '\0';
+ }
goto retry;
+ }
else
return -1;
}
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
