public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v6 3/4] Remove globals readSegNo, readOff, readLen 5+ messages / 2 participants [nested] [flat]
* [PATCH v6 3/4] Remove globals readSegNo, readOff, readLen @ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw) These global variables is functionally duplicated with them in XLogReaderState. Remove the globals. --- src/backend/access/transam/xlog.c | 77 ++++++++++++++++++--------------------- src/include/access/xlogreader.h | 2 +- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 34ab4ea359..c20d3b1418 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0; * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which - * will be just past that page. readLen indicates how much of the current - * page has been read into readBuf, and readSource indicates where we got - * the currently open file from. + * will be just past that page. readSource indicates where we got the + * currently open file from. */ static int readFile = -1; -static XLogSegNo readSegNo = 0; -static uint32 readOff = 0; -static uint32 readLen = 0; static XLogSource readSource = 0; /* XLOG_FROM_* code */ /* @@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli, int source, bool notfoundOk); static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source); -static bool XLogPageRead(XLogReaderState *xlogreader, +static bool XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess); static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr); + bool fetching_ckpt, + XLogRecPtr tliRecPtr, + XLogSegNo readSegNo); static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); static void XLogFileClose(void); static void PreallocXlogFiles(XLogRecPtr endptr); @@ -7494,7 +7492,8 @@ StartupXLOG(void) XLogRecPtr pageBeginPtr; pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ); - Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size)); + Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) == + XLogSegmentOffset(pageBeginPtr, wal_segment_size)); firstIdx = XLogRecPtrToBufIdx(EndOfLog); @@ -11514,14 +11513,15 @@ CancelBackup(void) * sleep and retry. */ static bool -XLogPageRead(XLogReaderState *xlogreader, +XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess) { - char *readBuf = xlogreader->readBuf; - XLogRecPtr targetPagePtr = xlogreader->readPagePtr; - int reqLen = xlogreader->readLen; - XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr; - TimeLineID *readTLI = &xlogreader->readPageTLI; + char *readBuf = state->readBuf; + XLogRecPtr targetPagePtr = state->readPagePtr; + int reqLen = state->readLen; + int readLen = 0; + XLogRecPtr targetRecPtr = state->ReadRecPtr; + TimeLineID *readTLI = &state->readPageTLI; uint32 targetPageOff; XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY; int r; @@ -11534,7 +11534,7 @@ XLogPageRead(XLogReaderState *xlogreader, * is not in the currently open one. */ if (readFile >= 0 && - !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size)) + !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size)) { /* * Request a restartpoint if we've replayed too much xlog since the @@ -11542,10 +11542,10 @@ XLogPageRead(XLogReaderState *xlogreader, */ if (bgwriterLaunched) { - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) { (void) GetRedoRecPtr(); - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) RequestCheckpoint(CHECKPOINT_CAUSE_XLOG); } } @@ -11555,7 +11555,7 @@ XLogPageRead(XLogReaderState *xlogreader, readSource = 0; } - XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size); + XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size); retry: /* See if we need to retrieve more data */ @@ -11564,17 +11564,14 @@ retry: receivedUpto < targetPagePtr + reqLen)) { if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen, - randAccess, - fetching_ckpt, - targetRecPtr)) + randAccess, fetching_ckpt, + targetRecPtr, state->readSegNo)) { if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; - - xlogreader->readLen = -1; + state->readLen = -1; return false; } } @@ -11602,37 +11599,33 @@ retry: else readLen = XLOG_BLCKSZ; - /* Read the requested page */ - readOff = targetPageOff; - pgstat_report_wait_start(WAIT_EVENT_WAL_READ); - r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); + r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff); if (r != XLOG_BLCKSZ) { char fname[MAXFNAMELEN]; int save_errno = errno; pgstat_report_wait_end(); - XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size); + XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size); if (r < 0) { errno = save_errno; ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode_for_file_access(), errmsg("could not read from log segment %s, offset %u: %m", - fname, readOff))); + fname, targetPageOff))); } else ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode(ERRCODE_DATA_CORRUPTED), errmsg("could not read from log segment %s, offset %u: read %d of %zu", - fname, readOff, r, (Size) XLOG_BLCKSZ))); + fname, targetPageOff, r, (Size) XLOG_BLCKSZ))); goto next_record_is_invalid; } pgstat_report_wait_end(); - Assert(targetSegNo == readSegNo); - Assert(targetPageOff == readOff); + Assert(targetSegNo == state->readSegNo); Assert(reqLen <= readLen); *readTLI = curFileTLI; @@ -11663,15 +11656,15 @@ retry: * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf)) { - /* reset any error XLogReaderValidatePageHeader() might have set */ - xlogreader->errormsg_buf[0] = '\0'; + /* reset any error StateValidatePageHeader() might have set */ + state->errormsg_buf[0] = '\0'; goto next_record_is_invalid; } - Assert(xlogreader->readPagePtr == targetPagePtr); - xlogreader->readLen = readLen; + Assert(state->readPagePtr == targetPagePtr); + state->readLen = readLen; return true; next_record_is_invalid: @@ -11680,14 +11673,13 @@ next_record_is_invalid: if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; /* In standby-mode, keep trying */ if (StandbyMode) goto retry; - xlogreader->readLen = -1; + state->readLen = -1; return false; } @@ -11719,7 +11711,8 @@ next_record_is_invalid: */ static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr) + bool fetching_ckpt, XLogRecPtr tliRecPtr, + XLogSegNo readSegNo) { static TimestampTz last_fail_time = 0; TimestampTz now; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 11ae82b96d..aca15be216 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -122,6 +122,7 @@ struct XLogReaderState * read by reader, which must be larger than * the request, or -1 on error */ TimeLineID readPageTLI; /* TLI for data currently in readBuf */ + XLogSegNo readSegNo; /* Segment # for data currently in readBuf */ char *readBuf; /* buffer to store data */ /* ---------------------------------------- @@ -152,7 +153,6 @@ struct XLogReaderState /* last read segment and segment offset for data currently in readBuf */ bool page_verified; bool record_verified; - XLogSegNo readSegNo; /* * beginning of prior page read, and its TLI. Doesn't necessarily -- 2.16.3 ----Next_Part(Tue_Sep_10_17_40_54_2019_177)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v6-0004-Change-policy-of-XLog-read-buffer-allocation.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v7 3/4] Remove globals readSegNo, readOff, readLen @ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw) These global variables is functionally duplicated with them in XLogReaderState. Remove the globals. --- src/backend/access/transam/xlog.c | 77 ++++++++++++++++++--------------------- src/include/access/xlogreader.h | 1 + 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index b06e381d73..49e8ca486e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0; * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which - * will be just past that page. readLen indicates how much of the current - * page has been read into readBuf, and readSource indicates where we got - * the currently open file from. + * will be just past that page. readSource indicates where we got the + * currently open file from. */ static int readFile = -1; -static XLogSegNo readSegNo = 0; -static uint32 readOff = 0; -static uint32 readLen = 0; static XLogSource readSource = 0; /* XLOG_FROM_* code */ /* @@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli, int source, bool notfoundOk); static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source); -static bool XLogPageRead(XLogReaderState *xlogreader, +static bool XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess); static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr); + bool fetching_ckpt, + XLogRecPtr tliRecPtr, + XLogSegNo readSegNo); static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); static void XLogFileClose(void); static void PreallocXlogFiles(XLogRecPtr endptr); @@ -7494,7 +7492,8 @@ StartupXLOG(void) XLogRecPtr pageBeginPtr; pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ); - Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size)); + Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) == + XLogSegmentOffset(pageBeginPtr, wal_segment_size)); firstIdx = XLogRecPtrToBufIdx(EndOfLog); @@ -11514,13 +11513,14 @@ CancelBackup(void) * sleep and retry. */ static bool -XLogPageRead(XLogReaderState *xlogreader, +XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess) { - char *readBuf = xlogreader->readBuf; - XLogRecPtr targetPagePtr = xlogreader->readPagePtr; - int reqLen = xlogreader->readLen; - XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr; + char *readBuf = state->readBuf; + XLogRecPtr targetPagePtr = state->readPagePtr; + int reqLen = state->readLen; + int readLen = 0; + XLogRecPtr targetRecPtr = state->ReadRecPtr; uint32 targetPageOff; XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY; int r; @@ -11533,7 +11533,7 @@ XLogPageRead(XLogReaderState *xlogreader, * is not in the currently open one. */ if (readFile >= 0 && - !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size)) + !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size)) { /* * Request a restartpoint if we've replayed too much xlog since the @@ -11541,10 +11541,10 @@ XLogPageRead(XLogReaderState *xlogreader, */ if (bgwriterLaunched) { - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) { (void) GetRedoRecPtr(); - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) RequestCheckpoint(CHECKPOINT_CAUSE_XLOG); } } @@ -11554,7 +11554,7 @@ XLogPageRead(XLogReaderState *xlogreader, readSource = 0; } - XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size); + XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size); retry: /* See if we need to retrieve more data */ @@ -11563,17 +11563,14 @@ retry: receivedUpto < targetPagePtr + reqLen)) { if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen, - randAccess, - fetching_ckpt, - targetRecPtr)) + randAccess, fetching_ckpt, + targetRecPtr, state->readSegNo)) { if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; - - xlogreader->readLen = -1; + state->readLen = -1; return false; } } @@ -11601,40 +11598,36 @@ retry: else readLen = XLOG_BLCKSZ; - /* Read the requested page */ - readOff = targetPageOff; - pgstat_report_wait_start(WAIT_EVENT_WAL_READ); - r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); + r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff); if (r != XLOG_BLCKSZ) { char fname[MAXFNAMELEN]; int save_errno = errno; pgstat_report_wait_end(); - XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size); + XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size); if (r < 0) { errno = save_errno; ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode_for_file_access(), errmsg("could not read from log segment %s, offset %u: %m", - fname, readOff))); + fname, targetPageOff))); } else ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode(ERRCODE_DATA_CORRUPTED), errmsg("could not read from log segment %s, offset %u: read %d of %zu", - fname, readOff, r, (Size) XLOG_BLCKSZ))); + fname, targetPageOff, r, (Size) XLOG_BLCKSZ))); goto next_record_is_invalid; } pgstat_report_wait_end(); - Assert(targetSegNo == readSegNo); - Assert(targetPageOff == readOff); + Assert(targetSegNo == state->readSegNo); Assert(reqLen <= readLen); - xlogreader->seg.ws_tli = curFileTLI; + state->seg.ws_tli = curFileTLI; /* * Check the page header immediately, so that we can retry immediately if @@ -11662,15 +11655,15 @@ retry: * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf)) { - /* reset any error XLogReaderValidatePageHeader() might have set */ - xlogreader->errormsg_buf[0] = '\0'; + /* reset any error StateValidatePageHeader() might have set */ + state->errormsg_buf[0] = '\0'; goto next_record_is_invalid; } - Assert(xlogreader->readPagePtr == targetPagePtr); - xlogreader->readLen = readLen; + Assert(state->readPagePtr == targetPagePtr); + state->readLen = readLen; return true; next_record_is_invalid: @@ -11679,14 +11672,13 @@ next_record_is_invalid: if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; /* In standby-mode, keep trying */ if (StandbyMode) goto retry; - xlogreader->readLen = -1; + state->readLen = -1; return false; } @@ -11718,7 +11710,8 @@ next_record_is_invalid: */ static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr) + bool fetching_ckpt, XLogRecPtr tliRecPtr, + XLogSegNo readSegNo) { static TimestampTz last_fail_time = 0; TimestampTz now; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 8e39b5c1f6..dc84d39f60 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -133,6 +133,7 @@ struct XLogReaderState * read by reader, which must be larger than * the request, or -1 on error */ TimeLineID readPageTLI; /* TLI for data currently in readBuf */ + XLogSegNo readSegNo; /* Segment # for data currently in readBuf */ char *readBuf; /* buffer to store data */ bool page_verified; /* is the page header on the buffer verified? */ bool record_verified;/* is the current record header verified? */ -- 2.16.3 ----Next_Part(Wed_Sep_25_15_50_32_2019_576)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v7-0004-Change-policy-of-XLog-read-buffer-allocation.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v8 3/4] Remove globals readSegNo, readOff, readLen @ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw) These global variables is functionally duplicated with them in XLogReaderState. Remove the globals. --- src/backend/access/transam/xlog.c | 77 ++++++++++++++++++--------------------- src/include/access/xlogreader.h | 1 + 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index b06e381d73..49e8ca486e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0; * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which - * will be just past that page. readLen indicates how much of the current - * page has been read into readBuf, and readSource indicates where we got - * the currently open file from. + * will be just past that page. readSource indicates where we got the + * currently open file from. */ static int readFile = -1; -static XLogSegNo readSegNo = 0; -static uint32 readOff = 0; -static uint32 readLen = 0; static XLogSource readSource = 0; /* XLOG_FROM_* code */ /* @@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli, int source, bool notfoundOk); static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source); -static bool XLogPageRead(XLogReaderState *xlogreader, +static bool XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess); static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr); + bool fetching_ckpt, + XLogRecPtr tliRecPtr, + XLogSegNo readSegNo); static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); static void XLogFileClose(void); static void PreallocXlogFiles(XLogRecPtr endptr); @@ -7494,7 +7492,8 @@ StartupXLOG(void) XLogRecPtr pageBeginPtr; pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ); - Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size)); + Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) == + XLogSegmentOffset(pageBeginPtr, wal_segment_size)); firstIdx = XLogRecPtrToBufIdx(EndOfLog); @@ -11514,13 +11513,14 @@ CancelBackup(void) * sleep and retry. */ static bool -XLogPageRead(XLogReaderState *xlogreader, +XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess) { - char *readBuf = xlogreader->readBuf; - XLogRecPtr targetPagePtr = xlogreader->readPagePtr; - int reqLen = xlogreader->readLen; - XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr; + char *readBuf = state->readBuf; + XLogRecPtr targetPagePtr = state->readPagePtr; + int reqLen = state->readLen; + int readLen = 0; + XLogRecPtr targetRecPtr = state->ReadRecPtr; uint32 targetPageOff; XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY; int r; @@ -11533,7 +11533,7 @@ XLogPageRead(XLogReaderState *xlogreader, * is not in the currently open one. */ if (readFile >= 0 && - !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size)) + !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size)) { /* * Request a restartpoint if we've replayed too much xlog since the @@ -11541,10 +11541,10 @@ XLogPageRead(XLogReaderState *xlogreader, */ if (bgwriterLaunched) { - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) { (void) GetRedoRecPtr(); - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) RequestCheckpoint(CHECKPOINT_CAUSE_XLOG); } } @@ -11554,7 +11554,7 @@ XLogPageRead(XLogReaderState *xlogreader, readSource = 0; } - XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size); + XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size); retry: /* See if we need to retrieve more data */ @@ -11563,17 +11563,14 @@ retry: receivedUpto < targetPagePtr + reqLen)) { if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen, - randAccess, - fetching_ckpt, - targetRecPtr)) + randAccess, fetching_ckpt, + targetRecPtr, state->readSegNo)) { if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; - - xlogreader->readLen = -1; + state->readLen = -1; return false; } } @@ -11601,40 +11598,36 @@ retry: else readLen = XLOG_BLCKSZ; - /* Read the requested page */ - readOff = targetPageOff; - pgstat_report_wait_start(WAIT_EVENT_WAL_READ); - r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); + r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff); if (r != XLOG_BLCKSZ) { char fname[MAXFNAMELEN]; int save_errno = errno; pgstat_report_wait_end(); - XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size); + XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size); if (r < 0) { errno = save_errno; ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode_for_file_access(), errmsg("could not read from log segment %s, offset %u: %m", - fname, readOff))); + fname, targetPageOff))); } else ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode(ERRCODE_DATA_CORRUPTED), errmsg("could not read from log segment %s, offset %u: read %d of %zu", - fname, readOff, r, (Size) XLOG_BLCKSZ))); + fname, targetPageOff, r, (Size) XLOG_BLCKSZ))); goto next_record_is_invalid; } pgstat_report_wait_end(); - Assert(targetSegNo == readSegNo); - Assert(targetPageOff == readOff); + Assert(targetSegNo == state->readSegNo); Assert(reqLen <= readLen); - xlogreader->seg.ws_tli = curFileTLI; + state->seg.ws_tli = curFileTLI; /* * Check the page header immediately, so that we can retry immediately if @@ -11662,15 +11655,15 @@ retry: * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf)) { - /* reset any error XLogReaderValidatePageHeader() might have set */ - xlogreader->errormsg_buf[0] = '\0'; + /* reset any error StateValidatePageHeader() might have set */ + state->errormsg_buf[0] = '\0'; goto next_record_is_invalid; } - Assert(xlogreader->readPagePtr == targetPagePtr); - xlogreader->readLen = readLen; + Assert(state->readPagePtr == targetPagePtr); + state->readLen = readLen; return true; next_record_is_invalid: @@ -11679,14 +11672,13 @@ next_record_is_invalid: if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; /* In standby-mode, keep trying */ if (StandbyMode) goto retry; - xlogreader->readLen = -1; + state->readLen = -1; return false; } @@ -11718,7 +11710,8 @@ next_record_is_invalid: */ static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr) + bool fetching_ckpt, XLogRecPtr tliRecPtr, + XLogSegNo readSegNo) { static TimestampTz last_fail_time = 0; TimestampTz now; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 8e39b5c1f6..dc84d39f60 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -133,6 +133,7 @@ struct XLogReaderState * read by reader, which must be larger than * the request, or -1 on error */ TimeLineID readPageTLI; /* TLI for data currently in readBuf */ + XLogSegNo readSegNo; /* Segment # for data currently in readBuf */ char *readBuf; /* buffer to store data */ bool page_verified; /* is the page header on the buffer verified? */ bool record_verified;/* is the current record header verified? */ -- 2.16.3 ----Next_Part(Fri_Sep_27_12_07_26_2019_308)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v8-0004-Change-policy-of-XLog-read-buffer-allocation.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v9 3/4] Remove globals readSegNo, readOff, readLen @ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw) These global variables is functionally duplicated with them in XLogReaderState. Remove the globals. --- src/backend/access/transam/xlog.c | 77 ++++++++++++++----------------- src/include/access/xlogreader.h | 1 + 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ce3dcbb399..e8a4c7916b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0; * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which - * will be just past that page. readLen indicates how much of the current - * page has been read into readBuf, and readSource indicates where we got - * the currently open file from. + * will be just past that page. readSource indicates where we got the + * currently open file from. */ static int readFile = -1; -static XLogSegNo readSegNo = 0; -static uint32 readOff = 0; -static uint32 readLen = 0; static XLogSource readSource = 0; /* XLOG_FROM_* code */ /* @@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli, int source, bool notfoundOk); static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source); -static bool XLogPageRead(XLogReaderState *xlogreader, +static bool XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess); static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr); + bool fetching_ckpt, + XLogRecPtr tliRecPtr, + XLogSegNo readSegNo); static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr); static void XLogFileClose(void); static void PreallocXlogFiles(XLogRecPtr endptr); @@ -7512,7 +7510,8 @@ StartupXLOG(void) XLogRecPtr pageBeginPtr; pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ); - Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size)); + Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) == + XLogSegmentOffset(pageBeginPtr, wal_segment_size)); firstIdx = XLogRecPtrToBufIdx(EndOfLog); @@ -11532,13 +11531,14 @@ CancelBackup(void) * sleep and retry. */ static bool -XLogPageRead(XLogReaderState *xlogreader, +XLogPageRead(XLogReaderState *state, bool fetching_ckpt, int emode, bool randAccess) { - char *readBuf = xlogreader->readBuf; - XLogRecPtr targetPagePtr = xlogreader->readPagePtr; - int reqLen = xlogreader->readLen; - XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr; + char *readBuf = state->readBuf; + XLogRecPtr targetPagePtr = state->readPagePtr; + int reqLen = state->readLen; + int readLen = 0; + XLogRecPtr targetRecPtr = state->ReadRecPtr; uint32 targetPageOff; XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY; int r; @@ -11551,7 +11551,7 @@ XLogPageRead(XLogReaderState *xlogreader, * is not in the currently open one. */ if (readFile >= 0 && - !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size)) + !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size)) { /* * Request a restartpoint if we've replayed too much xlog since the @@ -11559,10 +11559,10 @@ XLogPageRead(XLogReaderState *xlogreader, */ if (bgwriterLaunched) { - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) { (void) GetRedoRecPtr(); - if (XLogCheckpointNeeded(readSegNo)) + if (XLogCheckpointNeeded(state->readSegNo)) RequestCheckpoint(CHECKPOINT_CAUSE_XLOG); } } @@ -11572,7 +11572,7 @@ XLogPageRead(XLogReaderState *xlogreader, readSource = 0; } - XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size); + XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size); retry: /* See if we need to retrieve more data */ @@ -11581,17 +11581,14 @@ retry: receivedUpto < targetPagePtr + reqLen)) { if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen, - randAccess, - fetching_ckpt, - targetRecPtr)) + randAccess, fetching_ckpt, + targetRecPtr, state->readSegNo)) { if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; - - xlogreader->readLen = -1; + state->readLen = -1; return false; } } @@ -11619,40 +11616,36 @@ retry: else readLen = XLOG_BLCKSZ; - /* Read the requested page */ - readOff = targetPageOff; - pgstat_report_wait_start(WAIT_EVENT_WAL_READ); - r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); + r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff); if (r != XLOG_BLCKSZ) { char fname[MAXFNAMELEN]; int save_errno = errno; pgstat_report_wait_end(); - XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size); + XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size); if (r < 0) { errno = save_errno; ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode_for_file_access(), errmsg("could not read from log segment %s, offset %u: %m", - fname, readOff))); + fname, targetPageOff))); } else ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen), (errcode(ERRCODE_DATA_CORRUPTED), errmsg("could not read from log segment %s, offset %u: read %d of %zu", - fname, readOff, r, (Size) XLOG_BLCKSZ))); + fname, targetPageOff, r, (Size) XLOG_BLCKSZ))); goto next_record_is_invalid; } pgstat_report_wait_end(); - Assert(targetSegNo == readSegNo); - Assert(targetPageOff == readOff); + Assert(targetSegNo == state->readSegNo); Assert(reqLen <= readLen); - xlogreader->seg.ws_tli = curFileTLI; + state->seg.ws_tli = curFileTLI; /* * Check the page header immediately, so that we can retry immediately if @@ -11680,15 +11673,15 @@ retry: * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf)) { - /* reset any error XLogReaderValidatePageHeader() might have set */ - xlogreader->errormsg_buf[0] = '\0'; + /* reset any error StateValidatePageHeader() might have set */ + state->errormsg_buf[0] = '\0'; goto next_record_is_invalid; } - Assert(xlogreader->readPagePtr == targetPagePtr); - xlogreader->readLen = readLen; + Assert(state->readPagePtr == targetPagePtr); + state->readLen = readLen; return true; next_record_is_invalid: @@ -11697,14 +11690,13 @@ next_record_is_invalid: if (readFile >= 0) close(readFile); readFile = -1; - readLen = 0; readSource = 0; /* In standby-mode, keep trying */ if (StandbyMode) goto retry; - xlogreader->readLen = -1; + state->readLen = -1; return false; } @@ -11736,7 +11728,8 @@ next_record_is_invalid: */ static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, - bool fetching_ckpt, XLogRecPtr tliRecPtr) + bool fetching_ckpt, XLogRecPtr tliRecPtr, + XLogSegNo readSegNo) { static TimestampTz last_fail_time = 0; TimestampTz now; diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 8e39b5c1f6..dc84d39f60 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -133,6 +133,7 @@ struct XLogReaderState * read by reader, which must be larger than * the request, or -1 on error */ TimeLineID readPageTLI; /* TLI for data currently in readBuf */ + XLogSegNo readSegNo; /* Segment # for data currently in readBuf */ char *readBuf; /* buffer to store data */ bool page_verified; /* is the page header on the buffer verified? */ bool record_verified;/* is the current record header verified? */ -- 2.23.0 ----Next_Part(Thu_Oct_24_14_51_01_2019_720)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v9-0004-Change-policy-of-XLog-read-buffer-allocation.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Buffer locking is special (hints, checksums, AIO writes) @ 2026-02-06 21:18 Kirill Reshke <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Kirill Reshke @ 2026-02-06 21:18 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Melanie Plageman <[email protected]>; Heikki Linnakangas <[email protected]>; Noah Misch <[email protected]>; Matthias van de Meent <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Robert Haas <[email protected]>; Michael Paquier <[email protected]> On Tue, 3 Feb 2026 at 03:33, Andres Freund <[email protected]> wrote: > > Hi, > > On 2026-01-14 16:20:58 -0500, Andres Freund wrote: > > I'm now working on cleaning up the last two commits. The most crucial bit is > > to simplify what happens in MarkSharedBufferDirtyHint(), we afaict can delete > > the use of DELAY_CHKPT_START etc and just go to marking the buffer dirty first > > and then do the WAL logging, just like normal WAL logging. The previous order > > was only required because we were dirtying the page while holding only a > > shared lock, which did not conflict with the lock held by SyncBuffers() etc. > > I've been working on that. > > - A lot of what was special about MarkBufferDirtyHint() isn't needed anymore: > > - The "abnormal" order of WAL logging before marking the buffer dirty was > only needed because we marked buffers dirty. Which in turn was only needed > because setting hint bits didn't conflict with flushing the page. With > share-exclusive they do conflict, and we can switch to the normal order of > operations, where marking a buffer dirty makes checkpoint wait when the > buffer is encountered (due to wanting to flush the buffer but not getting > the lock) > > > - Now that we use the normal order of WAL logging, we don't need to delay > checkpoint starts anymore. > > I think the explanation for why that is ok is correct [1], but it needs to > be looked at by somebody with experience around this. Maybe Heikki? > > > - Thanks to holding share-exclusive lock, nothing can concurrently dirty or > undirty the buffer. Therefore the comments about spurious failures to mark > the buffer dirty can be removed. > > > - I realized that, now that buffers cannot be dirtied while IO is ongoing, we > don't need BM_JUST_DIRTIED anymore. > > > - The way MarkBufferDirtyHint() operates was copied into > heap_inplace_update_and_unlock(). Now that MarkBufferDirtyHint() won't work > that way anymore, it seems better to go with the alternative approach the > comments already outlined, namely to only delay updating of the buffer > contents. > > I've done this in a prequisite commit, as it doesn't actually depend on any > of the other changes. Noah, any chance you could take a look at this? > > > - Lots of minor polish > > > Greetings, > > Andres Freund > > [1] > /* > * Update RedoRecPtr so that we can make the right decision. It's possible > * that a new checkpoint will start just after GetRedoRecPtr(), but that > * is ok, as the buffer is already dirty, ensuring that any BufferSync() > * started after the buffer was marked dirty cannot complete without > * flushing this buffer. If a checkpoint started between marking the > * buffer dirty and this check, we will emit an unnecessary WAL record (as > * the buffer will be written out as part of the checkpoint), but the > * window for that is small. > */ Hi! I was reviewing new patches in this thread, and noticed your changes in v12-0002, in gistkillitems. This makes me think you can be interested in reviewing [0]. Anyway, v12-0003, on other patches I don't have an opinion (yet). [0] https://commitfest.postgresql.org/patch/6399/ -- Best regards, Kirill Reshke ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-02-06 21:18 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-09-10 08:28 [PATCH v9 3/4] Remove globals readSegNo, readOff, readLen Kyotaro Horiguchi <[email protected]> 2019-09-10 08:28 [PATCH v7 3/4] Remove globals readSegNo, readOff, readLen Kyotaro Horiguchi <[email protected]> 2019-09-10 08:28 [PATCH v6 3/4] Remove globals readSegNo, readOff, readLen Kyotaro Horiguchi <[email protected]> 2019-09-10 08:28 [PATCH v8 3/4] Remove globals readSegNo, readOff, readLen Kyotaro Horiguchi <[email protected]> 2026-02-06 21:18 Re: Buffer locking is special (hints, checksums, AIO writes) Kirill Reshke <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox