agora inbox for [email protected]help / color / mirror / Atom feed
Re: [HACKERS] Range Merge Join v1 23+ messages / 8 participants [nested] [flat]
* Re: [HACKERS] Range Merge Join v1 @ 2017-11-29 01:43 Michael Paquier <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Michael Paquier @ 2017-11-29 01:43 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: Alexander Kuzmenkov <[email protected]>; pgsql-hackers On Mon, Sep 18, 2017 at 2:24 AM, Jeff Davis <[email protected]> wrote: > Any comments or alternative suggestions welcome. This will probably > take a few days at least, so I put the patch in "waiting on author" > state. This did not receive an update for two months. I am marking it as returned with feedback. -- Michael ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3] Avoid creating archive status ".ready" files too early. @ 2021-07-30 22:35 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Alvaro Herrera @ 2021-07-30 22:35 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file will be created. If PostgreSQL crashes before it is able to write the rest of the record, it will end up reusing segments that have already been marked as ready-for-archival. However, the archiver process may have already processed the old version of the segment, so the wrong version of the segment may be backed-up. This backed-up segment will cause operations such as point-in-time restores to fail. To fix this, we keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 286 ++++++++++++++++++++++- src/backend/access/transam/xlogarchive.c | 14 +- src/backend/replication/walreceiver.c | 6 +- src/backend/storage/lmgr/lwlocknames.txt | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 5 + 7 files changed, 296 insertions(+), 22 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f84c0bb01e..99fe1ac0a2 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -512,6 +512,13 @@ typedef enum ExclusiveBackupState */ static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE; +/* entries for RecordBoundaryMap, used to mark segments ready for archival */ +typedef struct RecordBoundaryEntry +{ + XLogSegNo seg; /* must be first */ + XLogRecPtr pos; +} RecordBoundaryEntry; + /* * Shared state data for WAL insertion. */ @@ -723,6 +730,12 @@ typedef struct XLogCtlData */ XLogRecPtr lastFpwDisableRecPtr; + /* + * The last segment we've marked ready for archival. Protected by + * info_lck. + */ + XLogSegNo lastNotifiedSeg; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -736,6 +749,12 @@ static WALInsertLockPadded *WALInsertLocks = NULL; */ static ControlFileData *ControlFile = NULL; +/* + * Record boundary map, used for marking segments as ready for archival. + * Protected by ArchNotifyLock. + */ +static HTAB *RecordBoundaryMap = NULL; + /* * Calculate the amount of space left on the page after 'endptr'. Beware * multiple evaluation! @@ -962,6 +981,13 @@ static XLogRecPtr XLogBytePosToRecPtr(uint64 bytepos); static XLogRecPtr XLogBytePosToEndRecPtr(uint64 bytepos); static uint64 XLogRecPtrToBytePos(XLogRecPtr ptr); static void checkXLogConsistency(XLogReaderState *record); +static void RegisterRecordBoundaryEntry(XLogSegNo seg, XLogRecPtr pos); +static void NotifySegmentsReadyForArchive(void); +static XLogSegNo GetLastNotifiedSegment(void); +static void SetLastNotifiedSegment(XLogSegNo seg); +static void SetLastNotifiedSegmentIfInvalid(XLogSegNo seg); +static XLogSegNo GetLatestRecordBoundarySegment(void); +static void RemoveRecordBoundariesUpTo(XLogSegNo seg); static void WALInsertLockAcquire(void); static void WALInsertLockAcquireExclusive(void); @@ -1009,6 +1035,8 @@ XLogInsertRecord(XLogRecData *rdata, info == XLOG_SWITCH); XLogRecPtr StartPos; XLogRecPtr EndPos; + XLogSegNo StartSeg; + XLogSegNo EndSeg; bool prevDoPageWrites = doPageWrites; /* we assume that all of the record header is in the first chunk */ @@ -1167,10 +1195,33 @@ XLogInsertRecord(XLogRecData *rdata, SpinLockRelease(&XLogCtl->info_lck); } + /* + * Record the record boundary if we crossed the segment boundary. This is + * used to ensure that segments are not marked ready for archival before the + * entire record has been flushed to disk. + * + * Note that we do not use XLByteToPrevSeg() for determining the ending + * segment. This is done so that a record that fits perfectly into the end + * of the segment is marked ready for archival as soon as the flushed + * pointer jumps to the next segment. + */ + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + if (StartSeg != EndSeg && XLogArchivingActive()) + { + RegisterRecordBoundaryEntry(EndSeg, EndPos); + + /* + * There's a chance that the record was already flushed to disk and we + * missed marking segments as ready for archive, so try to do that now. + */ + NotifySegmentsReadyForArchive(); + } + /* * If this was an XLOG_SWITCH record, flush the record and the empty - * padding space that fills the rest of the segment, and perform - * end-of-segment actions (eg, notifying archiver). + * padding space that fills the rest of the segment. */ if (isLogSwitch) { @@ -1264,6 +1315,205 @@ XLogInsertRecord(XLogRecData *rdata, return EndPos; } +/* + * RegisterRecordBoundaryEntry + * + * This enters a new entry into the record boundary map, which is used for + * determing when it is safe to mark a segment as ready for archival. An entry + * with the given key (the segment number) must not already exist in the map. + * Also, the caller is responsible for ensuring that XLByteToSeg() would return + * the same segment number for the given record pointer. + */ +static void +RegisterRecordBoundaryEntry(XLogSegNo seg, XLogRecPtr pos) +{ + RecordBoundaryEntry *entry; + bool found; + + LWLockAcquire(ArchNotifyLock, LW_EXCLUSIVE); + + entry = (RecordBoundaryEntry *) hash_search(RecordBoundaryMap, + (void *) &seg, HASH_ENTER, + &found); + if (found) + elog(ERROR, "record boundary entry for segment already exists"); + + entry->pos = pos; + + LWLockRelease(ArchNotifyLock); +} + +/* + * NotifySegmentsReadyForArchive + * + * This function marks segments as ready for archival, given that it is safe to + * do so. It is safe to call this function repeatedly, even if nothing has + * changed since the last time it was called. + */ +static void +NotifySegmentsReadyForArchive(void) +{ + XLogRecPtr flushed; + XLogSegNo flushed_seg; + XLogSegNo latest_boundary_seg; + XLogSegNo last_notified; + + /* + * We first do a quick sanity check to see if we can bail out without taking + * the ArchNotifyLock at all. It is expected that this function will run + * frequently and that it will need to do nothing the vast majority of the + * time. + * + * Specifically, we bail out if the shared memory value for the last + * notified segment has not yet been initialized or if we've already marked + * the segment prior to the segment that contains "flushed" as ready for + * archival. We intentionally use XLByteToSeg() instead of + * XLByteToPrevSeg() so that we don't skip notifying when a record fits + * perfectly into the end of a segment. ("flushed" should point to the + * first byte of the record _after_ the one that is known to be flushed to + * disk.) + */ + last_notified = GetLastNotifiedSegment(); + + if (XLogSegNoIsInvalid(last_notified)) + return; + + flushed = GetFlushRecPtr(); + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + if (last_notified >= flushed_seg - 1) + return; + + /* + * Notify archiver about segments that are ready for archival (by creating + * the corresponding .ready files), and discard segment boundaries no + * longer needed. + */ + LWLockAcquire(ArchNotifyLock, LW_EXCLUSIVE); + latest_boundary_seg = GetLatestRecordBoundarySegment(); + if (!XLogSegNoIsInvalid(latest_boundary_seg)) + { + XLogSegNo seg; + + /* create the archive status files */ + for (seg = GetLastNotifiedSegment() + 1; seg < latest_boundary_seg; seg++) + XLogArchiveNotifySeg(seg, false); + + /* update shared memory */ + SetLastNotifiedSegment(latest_boundary_seg - 1); + + /* remove old boundaries from the map */ + RemoveRecordBoundariesUpTo(latest_boundary_seg); + + PgArchWakeup(); + } + LWLockRelease(ArchNotifyLock); +} + +/* + * GetLatestRecordBoundarySegment + * + * This function finds the latest record boundary in RecordBoundaryMap that is + * less than or equal to the current "flushed" pointer and returns its + * associated segment number, given that it is greater than the last notified + * segment. Otherwise, InvalidXLogSegNo is returned. + */ +static XLogSegNo +GetLatestRecordBoundarySegment(void) +{ + XLogRecPtr flushed; + XLogSegNo flushed_seg; + XLogSegNo last_notified; + XLogSegNo seg; + + Assert(LWLockHeldByMe(ArchNotifyLock)); + + flushed = GetFlushRecPtr(); + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + last_notified = GetLastNotifiedSegment(); + + for (seg = flushed_seg; seg > last_notified; seg--) + { + RecordBoundaryEntry *entry; + + entry = (RecordBoundaryEntry *) hash_search(RecordBoundaryMap, + (void *) &seg, HASH_FIND, + NULL); + + if (entry != NULL && flushed >= entry->pos) + return entry->seg; + } + + return InvalidXLogSegNo; +} + +/* + * RemoveOldRecordBoundaries + * + * This function removes all entries in the RecordBoundaryMap with segment + * numbers up to an including seg. + */ +static void +RemoveRecordBoundariesUpTo(XLogSegNo seg) +{ + RecordBoundaryEntry *entry; + HASH_SEQ_STATUS status; + + Assert(LWLockHeldByMeInMode(ArchNotifyLock, LW_EXCLUSIVE)); + + hash_seq_init(&status, RecordBoundaryMap); + + while ((entry = (RecordBoundaryEntry *) hash_seq_search(&status)) != NULL) + { + if (entry->seg <= seg) + (void) hash_search(RecordBoundaryMap, (void *) &entry->seg, + HASH_REMOVE, NULL); + } +} + +/* + * GetLastNotifiedSegment + * + * Retrieves last notified segment from shared memory. + */ +XLogSegNo +GetLastNotifiedSegment(void) +{ + XLogSegNo seg; + + SpinLockAcquire(&XLogCtl->info_lck); + seg = XLogCtl->lastNotifiedSeg; + SpinLockRelease(&XLogCtl->info_lck); + + return seg; +} + +/* + * SetLastNotifiedSegment + * + * Sets last notified segment in shared memory. + */ +static void +SetLastNotifiedSegment(XLogSegNo seg) +{ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->lastNotifiedSeg = seg; + SpinLockRelease(&XLogCtl->info_lck); +} + +/* + * SetLastNotifiedSegmentIfInvalid + * + * Sets last notified segment, but only if it's currently unset. + */ +static void +SetLastNotifiedSegmentIfInvalid(XLogSegNo seg) +{ + SpinLockAcquire(&XLogCtl->info_lck); + if (XLogCtl->lastNotifiedSeg == InvalidXLogSegNo) + XLogCtl->lastNotifiedSeg = seg; + SpinLockRelease(&XLogCtl->info_lck); +} + /* * Reserves the right amount of space for a record of given size from the WAL. * *StartPos is set to the beginning of the reserved section, *EndPos to @@ -2586,11 +2836,14 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * If WAL archiving is active and lastNotifiedSeg hasn't been + * initialized yet, do that, too. This will let us notify + * archiver correctly later. + * + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. + * */ if (finishing_seg) { @@ -2602,7 +2855,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + SetLastNotifiedSegmentIfInvalid(openLogSegNo - 1); XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2690,6 +2943,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(); } /* @@ -5117,6 +5373,9 @@ XLOGShmemSize(void) /* and the buffers themselves */ size = add_size(size, mul_size(XLOG_BLCKSZ, XLOGbuffers)); + /* stuff for marking segments as ready for archival */ + size = add_size(size, hash_estimate_size(16, sizeof(RecordBoundaryEntry))); + /* * Note: we don't count ControlFileData, it comes out of the "slop factor" * added by CreateSharedMemoryAndSemaphores. This lets us use this @@ -5134,6 +5393,7 @@ XLOGShmemInit(void) char *allocptr; int i; ControlFileData *localControlFile; + HASHCTL info; #ifdef WAL_DEBUG @@ -5227,12 +5487,20 @@ XLOGShmemInit(void) XLogCtl->InstallXLogFileSegmentActive = false; XLogCtl->SharedPromoteIsTriggered = false; XLogCtl->WalWriterSleeping = false; + XLogCtl->lastNotifiedSeg = InvalidXLogSegNo; SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(XLogSegNo); + info.entrysize = sizeof(RecordBoundaryEntry); + RecordBoundaryMap = ShmemInitHash("Record Boundary Table", 16, 16, &info, + HASH_ELEM | HASH_BLOBS); } /* @@ -7997,7 +8265,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..e9cac90b4a 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -464,7 +464,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * then when complete, rename it to 0000000100000001000000C6.done */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool notify) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +489,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, notify archiver that it's got something to do */ + if (notify) PgArchWakeup(); } @@ -498,12 +498,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool notify) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, notify); } /* @@ -608,7 +608,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt index 6c7cf6c295..d39225bf94 100644 --- a/src/backend/storage/lmgr/lwlocknames.txt +++ b/src/backend/storage/lmgr/lwlocknames.txt @@ -53,3 +53,4 @@ XactTruncationLock 44 # 45 was XactTruncationLock until removal of BackendRandomLock WrapLimitsVacuumLock 46 NotifyQueueTailLock 47 +ArchNotifyLock 48 diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..e25c3d8117 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool notify); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool notify); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..b5a0023d81 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -47,6 +47,11 @@ typedef uint64 XLogRecPtr; */ typedef uint64 XLogSegNo; +#define InvalidXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF) +#define XLogSegNoIsInvalid(s) \ + (AssertVariableIsOfTypeMacro(s, XLogSegNo), \ + (s) == InvalidXLogSegNo) + /* * TimeLineID (TLI) - identifies different database histories to prevent * confusion after restoring a prior state of a database installation. -- 2.20.1 --vp4c52all22p7wxm-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v2] Avoid creating archive status ".ready" files too early. @ 2021-07-30 22:35 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Alvaro Herrera @ 2021-07-30 22:35 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file will be created. If PostgreSQL crashes before it is able to write the rest of the record, it will end up reusing segments that have already been marked as ready-for-archival. However, the archiver process may have already processed the old version of the segment, so the wrong version of the segment may be backed-up. This backed-up segment will cause operations such as point-in-time restores to fail. To fix this, we keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 279 ++++++++++++++++++++++- src/backend/access/transam/xlogarchive.c | 14 +- src/backend/replication/walreceiver.c | 6 +- src/backend/storage/lmgr/lwlocknames.txt | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 3 + 7 files changed, 289 insertions(+), 20 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 26fa2b6c8f..700bbccff8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -531,6 +531,13 @@ typedef enum ExclusiveBackupState */ static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE; +/* entries for RecordBoundaryMap, used to mark segments ready for archival */ +typedef struct RecordBoundaryEntry +{ + XLogSegNo seg; /* must be first */ + XLogRecPtr pos; +} RecordBoundaryEntry; + /* * Shared state data for WAL insertion. */ @@ -742,6 +749,12 @@ typedef struct XLogCtlData */ XLogRecPtr lastFpwDisableRecPtr; + /* + * The last segment we've marked ready for archival. Protected by + * ArchNotifyLock. + */ + XLogSegNo lastNotifiedSeg; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -755,6 +768,12 @@ static WALInsertLockPadded *WALInsertLocks = NULL; */ static ControlFileData *ControlFile = NULL; +/* + * Record boundary map, used for marking segments as ready for archival. + * Protected by ArchNotifyLock. + */ +static HTAB *RecordBoundaryMap = NULL; + /* * Calculate the amount of space left on the page after 'endptr'. Beware * multiple evaluation! @@ -983,6 +1002,12 @@ static XLogRecPtr XLogBytePosToRecPtr(uint64 bytepos); static XLogRecPtr XLogBytePosToEndRecPtr(uint64 bytepos); static uint64 XLogRecPtrToBytePos(XLogRecPtr ptr); static void checkXLogConsistency(XLogReaderState *record); +static void RegisterRecordBoundaryEntry(XLogSegNo seg, XLogRecPtr pos); +static void NotifySegmentsReadyForArchive(void); +static XLogSegNo GetLastNotifiedSegment(void); +static void SetLastNotifiedSegment(XLogSegNo seg); +static XLogSegNo GetLatestRecordBoundarySegment(void); +static void RemoveRecordBoundariesUpTo(XLogSegNo seg); static void WALInsertLockAcquire(void); static void WALInsertLockAcquireExclusive(void); @@ -1030,6 +1055,8 @@ XLogInsertRecord(XLogRecData *rdata, info == XLOG_SWITCH); XLogRecPtr StartPos; XLogRecPtr EndPos; + XLogSegNo StartSeg; + XLogSegNo EndSeg; bool prevDoPageWrites = doPageWrites; /* we assume that all of the record header is in the first chunk */ @@ -1188,6 +1215,31 @@ XLogInsertRecord(XLogRecData *rdata, SpinLockRelease(&XLogCtl->info_lck); } + /* + * Record the record boundary if we crossed the segment boundary. This is + * used to ensure that segments are not marked ready for archival before the + * entire record has been flushed to disk. + * + * Note that we do not use XLByteToPrevSeg() for determining the ending + * segment. This is done so that a record that fits perfectly into the end + * of the segment is marked ready for archival as soon as the flushed + * pointer jumps to the next segment. + */ + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + if (StartSeg != EndSeg && + XLogArchivingActive()) + { + RegisterRecordBoundaryEntry(EndSeg, EndPos); + + /* + * There's a chance that the record was already flushed to disk and we + * missed marking segments as ready for archive, so try to do that now. + */ + NotifySegmentsReadyForArchive(); + } + /* * If this was an XLOG_SWITCH record, flush the record and the empty * padding space that fills the rest of the segment, and perform @@ -1285,6 +1337,197 @@ XLogInsertRecord(XLogRecData *rdata, return EndPos; } +/* + * RegisterRecordBoundaryEntry + * + * This enters a new entry into the record boundary map, which is used for + * determing when it is safe to mark a segment as ready for archival. An entry + * with the given key (the segment number) must not already exist in the map. + * Also, the caller is responsible for ensuring that XLByteToSeg() would return + * the same segment number for the given record pointer. + */ +static void +RegisterRecordBoundaryEntry(XLogSegNo seg, XLogRecPtr pos) +{ + RecordBoundaryEntry *entry; + bool found; + + LWLockAcquire(ArchNotifyLock, LW_EXCLUSIVE); + + entry = (RecordBoundaryEntry *) hash_search(RecordBoundaryMap, + (void *) &seg, HASH_ENTER, + &found); + if (found) + elog(ERROR, "record boundary entry for segment already exists"); + + entry->pos = pos; + + LWLockRelease(ArchNotifyLock); +} + +/* + * NotifySegmentsReadyForArchive + * + * This function marks segments as ready for archival, given that it is safe to + * do so. It is safe to call this function repeatedly, even if nothing has + * changed since the last time it was called. + */ +static void +NotifySegmentsReadyForArchive(void) +{ + XLogRecPtr flushed; + XLogSegNo flushed_seg; + XLogSegNo latest_boundary_seg; + XLogSegNo last_notified; + + /* + * We first do a quick sanity check to see if we can bail out without taking + * the ArchNotifyLock at all. It is expected that this function will run + * frequently and that it will need to do nothing the vast majority of the + * time. + * + * Specifically, we bail out if the shared memory value for the last + * notified segment has not yet been initialized or if we've already marked + * the segment prior to the segment that contains "flushed" as ready for + * archival. We intentionally use XLByteToSeg() instead of + * XLByteToPrevSeg() so that we don't skip notifying when a record fits + * perfectly into the end of a segment. ("flushed" should point to the + * first byte of the record _after_ the one that is known to be flushed to + * disk.) + */ + LWLockAcquire(ArchNotifyLock, LW_SHARED); + last_notified = GetLastNotifiedSegment(); + LWLockRelease(ArchNotifyLock); + + if (XLogSegNoIsInvalid(last_notified)) + return; + + flushed = GetFlushRecPtr(); + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + if (last_notified >= flushed_seg - 1) + return; + + /* + * At this point, we must acquire ArchNotifyLock before proceeding. In this + * section, we look for the latest record boundary in RecordBoundaryMap that + * is less than or equal to the current "flushed" pointer, and we notify the + * archiver that all segments up to (but not including) that boundary's + * associated segment are ready for archival. + */ + LWLockAcquire(ArchNotifyLock, LW_EXCLUSIVE); + + latest_boundary_seg = GetLatestRecordBoundarySegment(); + if (!XLogSegNoIsInvalid(latest_boundary_seg)) + { + XLogSegNo i; + + /* create the archive status files */ + for (i = GetLastNotifiedSegment() + 1; i < latest_boundary_seg; i++) + XLogArchiveNotifySeg(i, false); + + /* update shared memory */ + SetLastNotifiedSegment(latest_boundary_seg - 1); + + /* remove old boundaries from the map */ + RemoveRecordBoundariesUpTo(latest_boundary_seg); + + PgArchWakeup(); + } + + LWLockRelease(ArchNotifyLock); +} + +/* + * GetLatestRecordBoundarySegment + * + * This function finds the latest record boundary in RecordBoundaryMap that is + * less than or equal to the current "flushed" pointer and returns its + * associated segment number, given that it is greater than the last notified + * segment. Otherwise, InvalidXLogSegNo is returned. + * + * Caller is expected to be holding ArchNotifyLock. + */ +static XLogSegNo +GetLatestRecordBoundarySegment(void) +{ + XLogRecPtr flushed; + XLogSegNo flushed_seg; + XLogSegNo last_notified; + + flushed = GetFlushRecPtr(); + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + last_notified = GetLastNotifiedSegment(); + + for (XLogSegNo i = flushed_seg; i > last_notified; i--) + { + RecordBoundaryEntry *entry; + + entry = (RecordBoundaryEntry *) hash_search(RecordBoundaryMap, + (void *) &i, HASH_FIND, + NULL); + + if (entry != NULL && flushed >= entry->pos) + return entry->seg; + } + + return InvalidXLogSegNo; +} + +/* + * RemoveOldRecordBoundaries + * + * This function removes all entries in the RecordBoundaryMap with segment + * numbers up to an including seg. + * + * Caller is expected to be holding ArchNotifyLock. + */ +static void +RemoveRecordBoundariesUpTo(XLogSegNo seg) +{ + RecordBoundaryEntry *entry; + HASH_SEQ_STATUS status; + + hash_seq_init(&status, RecordBoundaryMap); + + while ((entry = (RecordBoundaryEntry *) hash_seq_search(&status)) != NULL) + { + if (entry->seg <= seg) + (void) hash_search(RecordBoundaryMap, (void *) &entry->seg, + HASH_REMOVE, NULL); + } +} + +/* + * GetLastNotifiedSegment + * + * Retrieves last notified segment from shared memory. + */ +XLogSegNo +GetLastNotifiedSegment(void) +{ + XLogSegNo seg; + + Assert(LWLockHeldByMe(ArchNotifyLock)); + + seg = XLogCtl->lastNotifiedSeg; + + return seg; +} + +/* + * SetLastNotifiedSegment + * + * Sets last notified segment in shared memory. Callers should hold + * ArchNotifyLock exclusively when calling this function. + */ +static void +SetLastNotifiedSegment(XLogSegNo seg) +{ + Assert(LWLockHeldByMeInMode(ArchNotifyLock, LW_EXCLUSIVE)); + + XLogCtl->lastNotifiedSeg = seg; +} + /* * Reserves the right amount of space for a record of given size from the WAL. * *StartPos is set to the beginning of the reserved section, *EndPos to @@ -2607,11 +2850,11 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. If + * lastNotifiedSeg hasn't been initialized yet, we need to do that, + * too. */ if (finishing_seg) { @@ -2623,7 +2866,14 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + { + LWLockAcquire(ArchNotifyLock, LW_EXCLUSIVE); + + if (XLogSegNoIsInvalid(GetLastNotifiedSegment())) + SetLastNotifiedSegment(openLogSegNo - 1); + + LWLockRelease(ArchNotifyLock); + } XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2711,6 +2961,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(); } /* @@ -5140,6 +5393,9 @@ XLOGShmemSize(void) /* and the buffers themselves */ size = add_size(size, mul_size(XLOG_BLCKSZ, XLOGbuffers)); + /* stuff for marking segments as ready for archival */ + size = add_size(size, hash_estimate_size(16, sizeof(RecordBoundaryEntry))); + /* * Note: we don't count ControlFileData, it comes out of the "slop factor" * added by CreateSharedMemoryAndSemaphores. This lets us use this @@ -5157,6 +5413,7 @@ XLOGShmemInit(void) char *allocptr; int i; ControlFileData *localControlFile; + HASHCTL info; #ifdef WAL_DEBUG @@ -5250,12 +5507,20 @@ XLOGShmemInit(void) XLogCtl->InstallXLogFileSegmentActive = false; XLogCtl->SharedPromoteIsTriggered = false; XLogCtl->WalWriterSleeping = false; + XLogCtl->lastNotifiedSeg = InvalidXLogSegNo; SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(XLogSegNo); + info.entrysize = sizeof(RecordBoundaryEntry); + RecordBoundaryMap = ShmemInitHash("Record Boundary Table", 16, 16, &info, + HASH_ELEM | HASH_BLOBS); } /* @@ -8016,7 +8281,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..e9cac90b4a 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -464,7 +464,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * then when complete, rename it to 0000000100000001000000C6.done */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool notify) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +489,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, notify archiver that it's got something to do */ + if (notify) PgArchWakeup(); } @@ -498,12 +498,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool notify) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, notify); } /* @@ -608,7 +608,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt index 6c7cf6c295..d39225bf94 100644 --- a/src/backend/storage/lmgr/lwlocknames.txt +++ b/src/backend/storage/lmgr/lwlocknames.txt @@ -53,3 +53,4 @@ XactTruncationLock 44 # 45 was XactTruncationLock until removal of BackendRandomLock WrapLimitsVacuumLock 46 NotifyQueueTailLock 47 +ArchNotifyLock 48 diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..e25c3d8117 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool notify); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool notify); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..8d172f887a 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -47,6 +47,9 @@ typedef uint64 XLogRecPtr; */ typedef uint64 XLogSegNo; +#define InvalidXLogSegNo 0xFFFFFFFFFFFFFFFF +#define XLogSegNoIsInvalid(s) ((s) == InvalidXLogSegNo) + /* * TimeLineID (TLI) - identifies different database histories to prevent * confusion after restoring a prior state of a database installation. -- 2.20.1 --mdy6umzbb7bbupyj-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v12] Avoid creating archive status ".ready" files too early @ 2021-08-17 03:52 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Nathan Bossart @ 2021-08-17 03:52 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file is created, and the archiver may process it. If PostgreSQL crashes before it is able to write and flush the rest of the record (in the next WAL segment), the wrong version of the first segment file lingers in the archive, which causes operations such as point-in-time restores to fail. To fix this, keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. XXX Note that this may add a new LWLock acquisition in the transaction commit path. (It doesn't happen in all cases.) Author: Nathan Bossart <[email protected]> Reviewed-by: Kyotaro Horiguchi <[email protected]> Reviewed-by: Ryo Matsumura <[email protected]> Reviewed-by: Andrey Borodin <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 307 ++++++++++++++++++++++- src/backend/access/transam/xlogarchive.c | 17 +- src/backend/postmaster/walwriter.c | 7 + src/backend/replication/walreceiver.c | 6 +- src/backend/storage/lmgr/lwlocknames.txt | 1 + src/include/access/xlog.h | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 1 + src/tools/pgindent/typedefs.list | 1 + 10 files changed, 323 insertions(+), 24 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e51a7a749d..01da4ab7b8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -512,6 +512,16 @@ typedef enum ExclusiveBackupState */ static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE; +/* + * Entries for SegmentBoundaryMap. Each such entry represents a WAL + * record that ends in endpos and crosses the WAL segment boundary. + */ +typedef struct SegmentBoundaryEntry +{ + XLogSegNo seg; /* must be first */ + XLogRecPtr endpos; +} SegmentBoundaryEntry; + /* * Shared state data for WAL insertion. */ @@ -723,6 +733,12 @@ typedef struct XLogCtlData */ XLogRecPtr lastFpwDisableRecPtr; + /* + * The last segment we've marked ready for archival. Protected by + * info_lck. + */ + XLogSegNo lastNotifiedSeg; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -736,6 +752,23 @@ static WALInsertLockPadded *WALInsertLocks = NULL; */ static ControlFileData *ControlFile = NULL; +/* + * Segment boundary map. + * + * This hash table tracks WAL records that cross WAL segment boundaries. + * For WAL archiving it is critical not to mark a segment as archivable + * until both halves of the record are flushed: otherwise if we crash + * after archiving the segment containing the first half and before + * flushing the second half of the record, then after recovery the + * primary would overwrite the first half with new data, making the + * segment archived prior to the crash a bogus copy. + * + * XXX note that an equivalent problem exists with streaming replication. + * + * Protected by SegmentBoundaryLock. + */ +static HTAB *SegmentBoundaryMap = NULL; + /* * Calculate the amount of space left on the page after 'endptr'. Beware * multiple evaluation! @@ -920,6 +953,13 @@ static void RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); +static void RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos); +static XLogSegNo GetLastNotifiedSegment(void); +static void SetLastNotifiedSegment(XLogSegNo seg); +static bool GetLatestSegmentBoundary(XLogSegNo last_notified, + XLogRecPtr flushed, + XLogSegNo *latest_boundary_seg); +static void RemoveSegmentBoundariesUpTo(XLogSegNo seg); static void CleanupBackupHistory(void); static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force); static XLogRecord *ReadRecord(XLogReaderState *xlogreader, @@ -1154,23 +1194,56 @@ XLogInsertRecord(XLogRecData *rdata, END_CRIT_SECTION(); /* - * Update shared LogwrtRqst.Write, if we crossed page boundary. + * If we crossed page boundary, update LogwrtRqst.Write; if we crossed + * segment boundary, register that and wake up walwriter. */ if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ) { + XLogSegNo StartSeg; + XLogSegNo EndSeg; + + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + /* + * Register our crossing the segment boundary if that occurred. + * + * Note that we did not use XLByteToPrevSeg() for determining the + * ending segment. This is so that a record that fits perfectly into + * the end of the segment is marked ready for archival as soon as the + * flushed pointer jumps to the next segment. + */ + if (StartSeg != EndSeg && XLogArchivingActive()) + RegisterSegmentBoundary(EndSeg, EndPos); + + /* + * Advance LogwrtRqst.Write so that it includes new block(s). + * + * We do this after registering the segment boundary so that the + * comparison with the flushed pointer below can use the latest value + * known globally. + */ SpinLockAcquire(&XLogCtl->info_lck); - /* advance global request to include new block(s) */ if (XLogCtl->LogwrtRqst.Write < EndPos) XLogCtl->LogwrtRqst.Write = EndPos; /* update local result copy while I have the chance */ LogwrtResult = XLogCtl->LogwrtResult; SpinLockRelease(&XLogCtl->info_lck); + + /* + * There's a chance that the record was already flushed to disk and we + * missed marking segments as ready for archive. If this happens, we + * nudge the WALWriter, which will take care of notifying segments as + * needed. + */ + if (StartSeg != EndSeg && XLogArchivingActive() && + LogwrtResult.Flush >= EndPos && ProcGlobal->walwriterLatch) + SetLatch(ProcGlobal->walwriterLatch); } /* * If this was an XLOG_SWITCH record, flush the record and the empty - * padding space that fills the rest of the segment, and perform - * end-of-segment actions (eg, notifying archiver). + * padding space that fills the rest of the segment. */ if (isLogSwitch) { @@ -2421,6 +2494,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) /* We should always be inside a critical section here */ Assert(CritSectionCount > 0); + Assert(LWLockHeldByMe(WALWriteLock)); /* * Update local LogwrtResult (caller probably did this already, but...) @@ -2586,11 +2660,13 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * If WAL archiving is active, we attempt to notify the archiver + * of any segments that are now ready for archival. + * + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. + * */ if (finishing_seg) { @@ -2602,7 +2678,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + NotifySegmentsReadyForArchive(LogwrtResult.Flush); XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2690,6 +2766,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(LogwrtResult.Flush); } /* @@ -4328,6 +4407,189 @@ ValidateXLOGDirectoryStructure(void) } } +/* + * RegisterSegmentBoundary + * Enter a new entry into the segment boundary map. + * + * An entry with the given key (the segment number) must not already exist in + * the map. + */ +static void +RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos) +{ + SegmentBoundaryEntry *entry; + XLogSegNo segno PG_USED_FOR_ASSERTS_ONLY; + bool found; + + /* verify caller computed segment number correctly */ + AssertArg((XLByteToSeg(pos, segno, wal_segment_size), segno == seg)); + + LWLockAcquire(SegmentBoundaryLock, LW_EXCLUSIVE); + + entry = (SegmentBoundaryEntry *) hash_search(SegmentBoundaryMap, + (void *) &seg, HASH_ENTER, + &found); + if (found) + elog(ERROR, "entry for segment already exists"); + + entry->endpos = pos; + + LWLockRelease(SegmentBoundaryLock); +} + +/* + * GetLatestSegmentBoundary + * + * Find the latest segment boundary in SegmentBoundaryMap that is less + * than or equal to the given "flushed" pointer and beyond the last + * notified segment. If such a segment is found, latest_boundary_seg + * is populated and true is returned. Otherwise, false is returned. + */ +static bool +GetLatestSegmentBoundary(XLogSegNo last_notified, XLogRecPtr flushed, + XLogSegNo *latest_boundary_seg) +{ + XLogSegNo flushed_seg; + XLogSegNo seg; + + Assert(LWLockHeldByMe(SegmentBoundaryLock)); + Assert(latest_boundary_seg != NULL); + + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + + for (seg = flushed_seg; seg > last_notified; seg--) + { + SegmentBoundaryEntry *entry; + + entry = (SegmentBoundaryEntry *) hash_search(SegmentBoundaryMap, + (void *) &seg, HASH_FIND, + NULL); + + if (entry != NULL && flushed >= entry->endpos) + { + *latest_boundary_seg = entry->seg; + return true; + } + } + + return false; +} + +/* + * RemoveSegmentBoundariesUpTo + * + * Remove all entries in the SegmentBoundaryMap with segment numbers + * up to and including seg. + */ +static void +RemoveSegmentBoundariesUpTo(XLogSegNo seg) +{ + SegmentBoundaryEntry *entry; + HASH_SEQ_STATUS status; + + Assert(LWLockHeldByMeInMode(SegmentBoundaryLock, LW_EXCLUSIVE)); + + hash_seq_init(&status, SegmentBoundaryMap); + + while ((entry = (SegmentBoundaryEntry *) hash_seq_search(&status)) != NULL) + { + if (entry->seg <= seg) + (void) hash_search(SegmentBoundaryMap, (void *) &entry->seg, + HASH_REMOVE, NULL); + } +} + +/* + * NotifySegmentsReadyForArchive + * + * Mark segments as ready for archival, given that it is safe to do so. + * This function is idempotent. + */ +void +NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr) +{ + XLogSegNo flushed_seg; + XLogSegNo latest_boundary_seg; + XLogSegNo last_notified; + + /* + * We first do a quick sanity check to see if we can bail out without + * taking the SegmentBoundaryLock at all. It is expected that this + * function will run frequently and that it will need to do nothing the + * vast majority of the time. + * + * Specifically, we bail out if we've already marked the segment prior to + * the segment that contains flushRecPtr as ready for archival. We + * intentionally use XLByteToSeg() instead of XLByteToPrevSeg() so that we + * don't skip notifying when a record fits perfectly into the end of a + * segment. (flushRecPtr should point to the first byte of the record + * _after_ the one that is known to be flushed to disk.) + */ + last_notified = GetLastNotifiedSegment(); + XLByteToSeg(flushRecPtr, flushed_seg, wal_segment_size); + if (last_notified >= flushed_seg - 1) + return; + + LWLockAcquire(SegmentBoundaryLock, LW_EXCLUSIVE); + + /* Reobtain lastNotifiedSeg in case someone else changed it. */ + last_notified = GetLastNotifiedSegment(); + + /* Retrieve the latest segment boundary to use for notifying segments. */ + if (GetLatestSegmentBoundary(last_notified, flushRecPtr, &latest_boundary_seg)) + { + /* + * Update shared memory and discard segment boundaries that are no + * longer needed. + * + * It is safe to update shared memory before we attempt to create the + * .ready files. If our calls to XLogArchiveNotifySeg() fail, + * RemoveOldXlogFiles() will retry it as needed. + */ + SetLastNotifiedSegment(latest_boundary_seg - 1); + RemoveSegmentBoundariesUpTo(latest_boundary_seg); + + LWLockRelease(SegmentBoundaryLock); + + /* + * Notify archiver about segments that are ready for archival (by + * creating the corresponding .ready files). + */ + for (XLogSegNo seg = last_notified + 1; seg < latest_boundary_seg; seg++) + XLogArchiveNotifySeg(seg, false); + + PgArchWakeup(); + } + else + LWLockRelease(SegmentBoundaryLock); +} + +/* + * GetLastNotifiedSegment + */ +XLogSegNo +GetLastNotifiedSegment(void) +{ + XLogSegNo seg; + + SpinLockAcquire(&XLogCtl->info_lck); + seg = XLogCtl->lastNotifiedSeg; + SpinLockRelease(&XLogCtl->info_lck); + + return seg; +} + +/* + * SetLastNotifiedSegment + */ +static void +SetLastNotifiedSegment(XLogSegNo seg) +{ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->lastNotifiedSeg = seg; + SpinLockRelease(&XLogCtl->info_lck); +} + /* * Remove previous backup history files. This also retries creation of * .ready files for any backup history files for which XLogArchiveNotify @@ -5117,6 +5379,9 @@ XLOGShmemSize(void) /* and the buffers themselves */ size = add_size(size, mul_size(XLOG_BLCKSZ, XLOGbuffers)); + /* hash table of segment-crossing WAL records */ + size = add_size(size, hash_estimate_size(16, sizeof(SegmentBoundaryEntry))); + /* * Note: we don't count ControlFileData, it comes out of the "slop factor" * added by CreateSharedMemoryAndSemaphores. This lets us use this @@ -5134,6 +5399,7 @@ XLOGShmemInit(void) char *allocptr; int i; ControlFileData *localControlFile; + HASHCTL info; #ifdef WAL_DEBUG @@ -5227,12 +5493,20 @@ XLOGShmemInit(void) XLogCtl->InstallXLogFileSegmentActive = false; XLogCtl->SharedPromoteIsTriggered = false; XLogCtl->WalWriterSleeping = false; + XLogCtl->lastNotifiedSeg = MaxXLogSegNo; SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(XLogSegNo); + info.entrysize = sizeof(SegmentBoundaryEntry); + SegmentBoundaryMap = ShmemInitHash("Segment Boundary Table", 16, 16, &info, + HASH_ELEM | HASH_BLOBS); } /* @@ -7873,6 +8147,17 @@ StartupXLOG(void) XLogCtl->LogwrtRqst.Write = EndOfLog; XLogCtl->LogwrtRqst.Flush = EndOfLog; + /* + * Initialize XLogCtl->lastNotifiedSeg to the previous WAL file. + */ + if (XLogArchivingActive()) + { + XLogSegNo EndOfLogSeg; + + XLByteToSeg(EndOfLog, EndOfLogSeg, wal_segment_size); + SetLastNotifiedSegment(EndOfLogSeg - 1); + } + /* * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE * record before resource manager writes cleanup WAL records or checkpoint @@ -8000,7 +8285,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..b9c19b2085 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -462,9 +462,12 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * by the archiver, e.g. we write 0000000100000001000000C6.ready * and the archiver then knows to archive XLOGDIR/0000000100000001000000C6, * then when complete, rename it to 0000000100000001000000C6.done + * + * Optionally, nudge the archiver process so that it'll notice the file we + * create. */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool nudge) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +492,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, let archiver know it's got work to do */ + if (nudge) PgArchWakeup(); } @@ -498,12 +501,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool nudge) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, nudge); } /* @@ -608,7 +611,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 626fae8454..6a1e16edc2 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -248,6 +248,13 @@ WalWriterMain(void) /* Process any signals received recently */ HandleWalWriterInterrupts(); + /* + * Notify the archiver of any WAL segments that are ready. We do this + * here to handle a race condition where WAL is flushed to disk prior + * to registering the segment boundary. + */ + NotifySegmentsReadyForArchive(GetFlushRecPtr()); + /* * Do what we're here for; then, if XLogBackgroundFlush() found useful * work to do, reset hibernation counter. diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt index 6c7cf6c295..14a742d655 100644 --- a/src/backend/storage/lmgr/lwlocknames.txt +++ b/src/backend/storage/lmgr/lwlocknames.txt @@ -53,3 +53,4 @@ XactTruncationLock 44 # 45 was XactTruncationLock until removal of BackendRandomLock WrapLimitsVacuumLock 46 NotifyQueueTailLock 47 +SegmentBoundaryLock 48 diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 0a8ede700d..6b6ae81c2d 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -315,6 +315,7 @@ extern XLogRecPtr GetInsertRecPtr(void); extern XLogRecPtr GetFlushRecPtr(void); extern XLogRecPtr GetLastImportantRecPtr(void); extern void RemovePromoteSignalFiles(void); +extern void NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr); extern bool PromoteIsTriggered(void); extern bool CheckPromoteSignal(void); diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..935b4cb02d 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool nudge); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool nudge); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..9b455e88e3 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -46,6 +46,7 @@ typedef uint64 XLogRecPtr; * XLogSegNo - physical log file sequence number. */ typedef uint64 XLogSegNo; +#define MaxXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF) /* * TimeLineID (TLI) - identifies different database histories to prevent diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 37cf4b2f76..79694b049e 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2334,6 +2334,7 @@ SecBufferDesc SecLabelItem SecLabelStmt SeenRelsEntry +SegmentBoundaryEntry SelectLimit SelectStmt Selectivity -- 2.20.1 --ngwxbmustuw56o2o-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v10] Avoid creating archive status ".ready" files too early. @ 2021-08-17 03:52 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Nathan Bossart @ 2021-08-17 03:52 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file will be created. If PostgreSQL crashes before it is able to write the rest of the record, it will end up reusing segments that have already been marked as ready-for-archival. However, the archiver process may have already processed the old version of the segment, so the wrong version of the segment may be backed-up. This backed-up segment will cause operations such as point-in-time restores to fail. To fix this, we keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 291 ++++++++++++++++++++++- src/backend/access/transam/xlogarchive.c | 17 +- src/backend/postmaster/walwriter.c | 7 + src/backend/replication/walreceiver.c | 6 +- src/backend/storage/lmgr/lwlocknames.txt | 1 + src/include/access/xlog.h | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 1 + src/tools/pgindent/typedefs.list | 1 + 10 files changed, 309 insertions(+), 22 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e51a7a749d..d2ccf2a7bb 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -512,6 +512,16 @@ typedef enum ExclusiveBackupState */ static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE; +/* + * Entries for SegmentBoundaryMap. Each such entry represents a WAL + * record that ends in endpos and crosses a WAL segment boundary. + */ +typedef struct SegmentBoundaryEntry +{ + XLogSegNo seg; /* must be first */ + XLogRecPtr endpos; +} SegmentBoundaryEntry; + /* * Shared state data for WAL insertion. */ @@ -723,6 +733,12 @@ typedef struct XLogCtlData */ XLogRecPtr lastFpwDisableRecPtr; + /* + * The last segment we've marked ready for archival. Protected by + * info_lck. + */ + XLogSegNo lastNotifiedSeg; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -736,6 +752,12 @@ static WALInsertLockPadded *WALInsertLocks = NULL; */ static ControlFileData *ControlFile = NULL; +/* + * Segment boundary map, used for marking segments as ready for archival. + * Protected by SegmentBoundaryLock. + */ +static HTAB *SegmentBoundaryMap = NULL; + /* * Calculate the amount of space left on the page after 'endptr'. Beware * multiple evaluation! @@ -962,6 +984,13 @@ static XLogRecPtr XLogBytePosToRecPtr(uint64 bytepos); static XLogRecPtr XLogBytePosToEndRecPtr(uint64 bytepos); static uint64 XLogRecPtrToBytePos(XLogRecPtr ptr); static void checkXLogConsistency(XLogReaderState *record); +static void RegisterSegmentBoundaryEntry(XLogSegNo seg, XLogRecPtr pos); +static XLogSegNo GetLastNotifiedSegment(void); +static void SetLastNotifiedSegment(XLogSegNo seg); +static bool GetLatestSegmentBoundary(XLogSegNo last_notified, + XLogRecPtr flushed, + XLogSegNo *latest_boundary_seg); +static void RemoveSegmentBoundariesUpTo(XLogSegNo seg); static void WALInsertLockAcquire(void); static void WALInsertLockAcquireExclusive(void); @@ -1158,6 +1187,9 @@ XLogInsertRecord(XLogRecData *rdata, */ if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ) { + XLogSegNo StartSeg; + XLogSegNo EndSeg; + SpinLockAcquire(&XLogCtl->info_lck); /* advance global request to include new block(s) */ if (XLogCtl->LogwrtRqst.Write < EndPos) @@ -1165,12 +1197,38 @@ XLogInsertRecord(XLogRecData *rdata, /* update local result copy while I have the chance */ LogwrtResult = XLogCtl->LogwrtResult; SpinLockRelease(&XLogCtl->info_lck); + + /* + * If we crossed the segment boundary, record it. This is used to + * ensure that segments are not marked ready for archival before the + * entire record has been flushed to disk. + * + * Note that we do not use XLByteToPrevSeg() for determining the + * ending segment. This is done so that a record that fits perfectly + * into the end of the segment is marked ready for archival as soon as + * the flushed pointer jumps to the next segment. + */ + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + if (StartSeg != EndSeg && XLogArchivingActive()) + { + RegisterSegmentBoundaryEntry(EndSeg, EndPos); + + /* + * There's a chance that the record was already flushed to disk + * and we missed marking segments as ready for archive. If this + * happens, we nudge the WALWriter, which will take care of + * notifying segments as needed. + */ + if (LogwrtResult.Flush > EndPos && ProcGlobal->walwriterLatch) + SetLatch(ProcGlobal->walwriterLatch); + } } /* * If this was an XLOG_SWITCH record, flush the record and the empty - * padding space that fills the rest of the segment, and perform - * end-of-segment actions (eg, notifying archiver). + * padding space that fills the rest of the segment. */ if (isLogSwitch) { @@ -1264,6 +1322,192 @@ XLogInsertRecord(XLogRecData *rdata, return EndPos; } +/* + * RegisterSegmentBoundaryEntry + * + * This enters a new entry into the segment boundary map, which is used for + * determing when it is safe to mark a segment as ready for archival. An entry + * with the given key (the segment number) must not already exist in the map. + * Also, the caller is responsible for ensuring that XLByteToSeg() would return + * the same segment number for the given record pointer. + */ +static void +RegisterSegmentBoundaryEntry(XLogSegNo seg, XLogRecPtr pos) +{ + SegmentBoundaryEntry *entry; + bool found; + + LWLockAcquire(SegmentBoundaryLock, LW_EXCLUSIVE); + + entry = (SegmentBoundaryEntry *) hash_search(SegmentBoundaryMap, + (void *) &seg, HASH_ENTER, + &found); + if (found) + elog(ERROR, "entry for segment already exists"); + + entry->endpos = pos; + + LWLockRelease(SegmentBoundaryLock); +} + +/* + * NotifySegmentsReadyForArchive + * + * This function marks segments as ready for archival, given that it is safe to + * do so. It is safe to call this function repeatedly, even if nothing has + * changed since the last time it was called. + */ +void +NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr) +{ + XLogSegNo flushed_seg; + XLogSegNo latest_boundary_seg; + XLogSegNo last_notified; + + /* + * We first do a quick sanity check to see if we can bail out without + * taking the SegmentBoundaryLock at all. It is expected that this + * function will run frequently and that it will need to do nothing the + * vast majority of the time. + * + * Specifically, we bail out if we've already marked the segment prior to + * the segment that contains flushRecPtr as ready for archival. We + * intentionally use XLByteToSeg() instead of XLByteToPrevSeg() so that we + * don't skip notifying when a record fits perfectly into the end of a + * segment. (flushRecPtr should point to the first byte of the record + * _after_ the one that is known to be flushed to disk.) + */ + last_notified = GetLastNotifiedSegment(); + XLByteToSeg(flushRecPtr, flushed_seg, wal_segment_size); + if (last_notified >= flushed_seg - 1) + return; + + LWLockAcquire(SegmentBoundaryLock, LW_EXCLUSIVE); + + /* Reobtain lastNotifiedSeg in case someone else changed it. */ + last_notified = GetLastNotifiedSegment(); + + /* Retrieve the latest segment boundary to use for notifying segments. */ + if (GetLatestSegmentBoundary(last_notified, flushRecPtr, &latest_boundary_seg)) + { + /* + * Update shared memory and discard segment boundaries that are no + * longer needed. + * + * It is safe to update shared memory before we attempt to create the + * .ready files. If our calls to XLogArchiveNotifySeg() fail, + * RemoveOldXlogFiles() will retry it as needed. + */ + SetLastNotifiedSegment(latest_boundary_seg - 1); + RemoveSegmentBoundariesUpTo(latest_boundary_seg); + + LWLockRelease(SegmentBoundaryLock); + + /* + * Notify archiver about segments that are ready for archival (by + * creating the corresponding .ready files). + */ + for (XLogSegNo seg = last_notified + 1; seg < latest_boundary_seg; seg++) + XLogArchiveNotifySeg(seg, false); + + PgArchWakeup(); + } + else + LWLockRelease(SegmentBoundaryLock); +} + +/* + * GetLatestSegmentBoundary + * + * This function finds the latest segment boundary in SegmentBoundaryMap that is + * less than or equal to the given "flushed" pointer and beyond the last + * notified segment. If such a segment is found, latest_boundary_seg is + * populated and true is returned. Otherwise, false is returned. + */ +static bool +GetLatestSegmentBoundary(XLogSegNo last_notified, XLogRecPtr flushed, + XLogSegNo *latest_boundary_seg) +{ + XLogSegNo flushed_seg; + XLogSegNo seg; + + Assert(LWLockHeldByMe(SegmentBoundaryLock)); + Assert(latest_boundary_seg != NULL); + + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + + for (seg = flushed_seg; seg > last_notified; seg--) + { + SegmentBoundaryEntry *entry; + + entry = (SegmentBoundaryEntry *) hash_search(SegmentBoundaryMap, + (void *) &seg, HASH_FIND, + NULL); + + if (entry != NULL && flushed >= entry->endpos) + { + *latest_boundary_seg = entry->seg; + return true; + } + } + + return false; +} + +/* + * RemoveSegmentBoundariesUpTo + * + * This function removes all entries in the SegmentBoundaryMap with segment + * numbers up to and including seg. + */ +static void +RemoveSegmentBoundariesUpTo(XLogSegNo seg) +{ + SegmentBoundaryEntry *entry; + HASH_SEQ_STATUS status; + + Assert(LWLockHeldByMeInMode(SegmentBoundaryLock, LW_EXCLUSIVE)); + + hash_seq_init(&status, SegmentBoundaryMap); + + while ((entry = (SegmentBoundaryEntry *) hash_seq_search(&status)) != NULL) + { + if (entry->seg <= seg) + (void) hash_search(SegmentBoundaryMap, (void *) &entry->seg, + HASH_REMOVE, NULL); + } +} + +/* + * GetLastNotifiedSegment + * + * Retrieves last notified segment from shared memory. + */ +XLogSegNo +GetLastNotifiedSegment(void) +{ + XLogSegNo seg; + + SpinLockAcquire(&XLogCtl->info_lck); + seg = XLogCtl->lastNotifiedSeg; + SpinLockRelease(&XLogCtl->info_lck); + + return seg; +} + +/* + * SetLastNotifiedSegment + * + * Sets last notified segment in shared memory. + */ +static void +SetLastNotifiedSegment(XLogSegNo seg) +{ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->lastNotifiedSeg = seg; + SpinLockRelease(&XLogCtl->info_lck); +} + /* * Reserves the right amount of space for a record of given size from the WAL. * *StartPos is set to the beginning of the reserved section, *EndPos to @@ -2421,6 +2665,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) /* We should always be inside a critical section here */ Assert(CritSectionCount > 0); + Assert(LWLockHeldByMe(WALWriteLock)); /* * Update local LogwrtResult (caller probably did this already, but...) @@ -2586,11 +2831,13 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * If WAL archiving is active, we attempt to notify the archiver + * of any segments that are now ready for archival. + * + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. + * */ if (finishing_seg) { @@ -2602,7 +2849,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + NotifySegmentsReadyForArchive(LogwrtResult.Flush); XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2690,6 +2937,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(LogwrtResult.Flush); } /* @@ -5117,6 +5367,9 @@ XLOGShmemSize(void) /* and the buffers themselves */ size = add_size(size, mul_size(XLOG_BLCKSZ, XLOGbuffers)); + /* stuff for marking segments as ready for archival */ + size = add_size(size, hash_estimate_size(16, sizeof(SegmentBoundaryEntry))); + /* * Note: we don't count ControlFileData, it comes out of the "slop factor" * added by CreateSharedMemoryAndSemaphores. This lets us use this @@ -5134,6 +5387,7 @@ XLOGShmemInit(void) char *allocptr; int i; ControlFileData *localControlFile; + HASHCTL info; #ifdef WAL_DEBUG @@ -5227,12 +5481,20 @@ XLOGShmemInit(void) XLogCtl->InstallXLogFileSegmentActive = false; XLogCtl->SharedPromoteIsTriggered = false; XLogCtl->WalWriterSleeping = false; + XLogCtl->lastNotifiedSeg = MaxXLogSegNo; SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(XLogSegNo); + info.entrysize = sizeof(SegmentBoundaryEntry); + SegmentBoundaryMap = ShmemInitHash("Segment Boundary Table", 16, 16, &info, + HASH_ELEM | HASH_BLOBS); } /* @@ -7873,6 +8135,17 @@ StartupXLOG(void) XLogCtl->LogwrtRqst.Write = EndOfLog; XLogCtl->LogwrtRqst.Flush = EndOfLog; + /* + * Initialize XLogCtl->lastNotifiedSeg to the previous WAL file. + */ + if (XLogArchivingActive()) + { + XLogSegNo EndOfLogSeg; + + XLByteToSeg(EndOfLog, EndOfLogSeg, wal_segment_size); + SetLastNotifiedSegment(EndOfLogSeg - 1); + } + /* * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE * record before resource manager writes cleanup WAL records or checkpoint @@ -8000,7 +8273,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..b9c19b2085 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -462,9 +462,12 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * by the archiver, e.g. we write 0000000100000001000000C6.ready * and the archiver then knows to archive XLOGDIR/0000000100000001000000C6, * then when complete, rename it to 0000000100000001000000C6.done + * + * Optionally, nudge the archiver process so that it'll notice the file we + * create. */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool nudge) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +492,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, let archiver know it's got work to do */ + if (nudge) PgArchWakeup(); } @@ -498,12 +501,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool nudge) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, nudge); } /* @@ -608,7 +611,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 626fae8454..6a1e16edc2 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -248,6 +248,13 @@ WalWriterMain(void) /* Process any signals received recently */ HandleWalWriterInterrupts(); + /* + * Notify the archiver of any WAL segments that are ready. We do this + * here to handle a race condition where WAL is flushed to disk prior + * to registering the segment boundary. + */ + NotifySegmentsReadyForArchive(GetFlushRecPtr()); + /* * Do what we're here for; then, if XLogBackgroundFlush() found useful * work to do, reset hibernation counter. diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/backend/storage/lmgr/lwlocknames.txt b/src/backend/storage/lmgr/lwlocknames.txt index 6c7cf6c295..14a742d655 100644 --- a/src/backend/storage/lmgr/lwlocknames.txt +++ b/src/backend/storage/lmgr/lwlocknames.txt @@ -53,3 +53,4 @@ XactTruncationLock 44 # 45 was XactTruncationLock until removal of BackendRandomLock WrapLimitsVacuumLock 46 NotifyQueueTailLock 47 +SegmentBoundaryLock 48 diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 0a8ede700d..6b6ae81c2d 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -315,6 +315,7 @@ extern XLogRecPtr GetInsertRecPtr(void); extern XLogRecPtr GetFlushRecPtr(void); extern XLogRecPtr GetLastImportantRecPtr(void); extern void RemovePromoteSignalFiles(void); +extern void NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr); extern bool PromoteIsTriggered(void); extern bool CheckPromoteSignal(void); diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..935b4cb02d 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool nudge); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool nudge); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..9b455e88e3 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -46,6 +46,7 @@ typedef uint64 XLogRecPtr; * XLogSegNo - physical log file sequence number. */ typedef uint64 XLogSegNo; +#define MaxXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF) /* * TimeLineID (TLI) - identifies different database histories to prevent diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 37cf4b2f76..79694b049e 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2334,6 +2334,7 @@ SecBufferDesc SecLabelItem SecLabelStmt SeenRelsEntry +SegmentBoundaryEntry SelectLimit SelectStmt Selectivity -- 2.30.2 --aabamu5eazcf6rc5-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v14] Avoid creating archive status ".ready" files too early @ 2021-08-20 19:25 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Nathan Bossart @ 2021-08-20 19:25 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file is created, and the archiver may process it. If PostgreSQL crashes before it is able to write and flush the rest of the record (in the next WAL segment), the wrong version of the first segment file lingers in the archive, which causes operations such as point-in-time restores to fail. To fix this, keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. Author: Nathan Bossart <[email protected]> Reviewed-by: Kyotaro Horiguchi <[email protected]> Reviewed-by: Ryo Matsumura <[email protected]> Reviewed-by: Andrey Borodin <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 242 +++++++++++++++++++++-- src/backend/access/transam/xlogarchive.c | 17 +- src/backend/postmaster/walwriter.c | 7 + src/backend/replication/walreceiver.c | 6 +- src/include/access/xlog.h | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 1 + 8 files changed, 256 insertions(+), 24 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e51a7a749d..95f03adef8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -724,6 +724,18 @@ typedef struct XLogCtlData XLogRecPtr lastFpwDisableRecPtr; slock_t info_lck; /* locks shared variables shown above */ + + /* + * Variables used to track cross-segment records. Protected by + * segtrack_lck. + */ + XLogSegNo lastNotifiedSeg; + XLogSegNo earliestSegBoundary; + XLogRecPtr earliestSegBoundaryRecPtr; + XLogSegNo latestSegBoundary; + XLogRecPtr latestSegBoundaryRecPtr; + + slock_t segtrack_lck; /* locks shared variables shown above */ } XLogCtlData; static XLogCtlData *XLogCtl = NULL; @@ -920,6 +932,9 @@ static void RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); +static void RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos); +static bool GetLatestSegmentBoundary(XLogRecPtr flushed, XLogSegNo *latest_boundary_seg); +static void RemoveSegmentBoundariesUpTo(XLogSegNo seg); static void CleanupBackupHistory(void); static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force); static XLogRecord *ReadRecord(XLogReaderState *xlogreader, @@ -1154,23 +1169,56 @@ XLogInsertRecord(XLogRecData *rdata, END_CRIT_SECTION(); /* - * Update shared LogwrtRqst.Write, if we crossed page boundary. + * If we crossed page boundary, update LogwrtRqst.Write; if we crossed + * segment boundary, register that and wake up walwriter. */ if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ) { + XLogSegNo StartSeg; + XLogSegNo EndSeg; + + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + /* + * Register our crossing the segment boundary if that occurred. + * + * Note that we did not use XLByteToPrevSeg() for determining the + * ending segment. This is so that a record that fits perfectly into + * the end of the segment is marked ready for archival as soon as the + * flushed pointer jumps to the next segment. + */ + if (StartSeg != EndSeg && XLogArchivingActive()) + RegisterSegmentBoundary(EndSeg, EndPos); + + /* + * Advance LogwrtRqst.Write so that it includes new block(s). + * + * We do this after registering the segment boundary so that the + * comparison with the flushed pointer below can use the latest value + * known globally. + */ SpinLockAcquire(&XLogCtl->info_lck); - /* advance global request to include new block(s) */ if (XLogCtl->LogwrtRqst.Write < EndPos) XLogCtl->LogwrtRqst.Write = EndPos; /* update local result copy while I have the chance */ LogwrtResult = XLogCtl->LogwrtResult; SpinLockRelease(&XLogCtl->info_lck); + + /* + * There's a chance that the record was already flushed to disk and we + * missed marking segments as ready for archive. If this happens, we + * nudge the WALWriter, which will take care of notifying segments as + * needed. + */ + if (StartSeg != EndSeg && XLogArchivingActive() && + LogwrtResult.Flush >= EndPos && ProcGlobal->walwriterLatch) + SetLatch(ProcGlobal->walwriterLatch); } /* * If this was an XLOG_SWITCH record, flush the record and the empty - * padding space that fills the rest of the segment, and perform - * end-of-segment actions (eg, notifying archiver). + * padding space that fills the rest of the segment. */ if (isLogSwitch) { @@ -2421,6 +2469,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) /* We should always be inside a critical section here */ Assert(CritSectionCount > 0); + Assert(LWLockHeldByMe(WALWriteLock)); /* * Update local LogwrtResult (caller probably did this already, but...) @@ -2586,11 +2635,12 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * If WAL archiving is active, we attempt to notify the archiver + * of any segments that are now ready for archival. + * + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. */ if (finishing_seg) { @@ -2602,7 +2652,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + NotifySegmentsReadyForArchive(LogwrtResult.Flush); XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2690,6 +2740,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(LogwrtResult.Flush); } /* @@ -4328,6 +4381,151 @@ ValidateXLOGDirectoryStructure(void) } } +/* + * RegisterSegmentBoundary + * Record a new segment boundary if needed. + */ +static void +RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos) +{ + XLogSegNo segno PG_USED_FOR_ASSERTS_ONLY; + + /* verify caller computed segment number correctly */ + AssertArg((XLByteToSeg(pos, segno, wal_segment_size), segno == seg)); + + SpinLockAcquire(&XLogCtl->segtrack_lck); + + /* + * If no segment boundaries are registered, store the new segment boundary + * in earliestSegBoundary. Otherwise, store the greater segment boundaries in + * latestSegBoundary. + */ + if (XLogCtl->earliestSegBoundary == MaxXLogSegNo) + { + XLogCtl->earliestSegBoundary = seg; + XLogCtl->earliestSegBoundaryRecPtr = pos; + } + else if (seg > XLogCtl->earliestSegBoundary && + (XLogCtl->latestSegBoundary == MaxXLogSegNo || + seg > XLogCtl->latestSegBoundary)) + { + XLogCtl->latestSegBoundary = seg; + XLogCtl->latestSegBoundaryRecPtr = pos; + } + + SpinLockRelease(&XLogCtl->segtrack_lck); +} + +/* + * GetLatestSegmentBoundary + * + * Look up the latest segment boundary that is less than or equal to the + * given "flushed" pointer. If such a segment is found, + * latest_boundary_seg is populated and true is returned. Otherwise, false + * is returned. + * + * Callers should hold XLogCtl->segtrack_lck when calling this function. + */ +static bool +GetLatestSegmentBoundary(XLogRecPtr flushed, XLogSegNo *latest_boundary_seg) +{ + XLogSegNo flushed_seg; + + Assert(latest_boundary_seg != NULL); + + XLByteToSeg(flushed, flushed_seg, wal_segment_size); + + if (XLogCtl->latestSegBoundary <= flushed_seg && + XLogCtl->latestSegBoundaryRecPtr <= flushed) + { + *latest_boundary_seg = XLogCtl->latestSegBoundary; + return true; + } + + if (XLogCtl->earliestSegBoundary <= flushed_seg && + XLogCtl->earliestSegBoundaryRecPtr <= flushed) + { + *latest_boundary_seg = XLogCtl->earliestSegBoundary; + return true; + } + + return false; +} + +/* + * RemoveSegmentBoundariesUpTo + * + * Remove all segment boundaries with segment numbers up to and including + * seg. + * + * Callers should hold XLogCtl->segtrack_lck when calling this function. + */ +static void +RemoveSegmentBoundariesUpTo(XLogSegNo seg) +{ + if (XLogCtl->latestSegBoundary <= seg) + { + XLogCtl->earliestSegBoundary = MaxXLogSegNo; + XLogCtl->earliestSegBoundaryRecPtr = InvalidXLogRecPtr; + + XLogCtl->latestSegBoundary = MaxXLogSegNo; + XLogCtl->latestSegBoundaryRecPtr = InvalidXLogRecPtr; + } + else if (XLogCtl->earliestSegBoundary <= seg) + { + XLogCtl->earliestSegBoundary = XLogCtl->latestSegBoundary; + XLogCtl->earliestSegBoundaryRecPtr = XLogCtl->latestSegBoundaryRecPtr; + + XLogCtl->latestSegBoundary = MaxXLogSegNo; + XLogCtl->latestSegBoundaryRecPtr = InvalidXLogRecPtr; + } +} + +/* + * NotifySegmentsReadyForArchive + * + * Mark segments as ready for archival, given that it is safe to do so. + * This function is idempotent. + */ +void +NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr) +{ + XLogSegNo latest_boundary_seg = 0; + + SpinLockAcquire(&XLogCtl->segtrack_lck); + + /* Retrieve the latest segment boundary to use for notifying segments. */ + if (GetLatestSegmentBoundary(flushRecPtr, &latest_boundary_seg)) + { + XLogSegNo last_notified = XLogCtl->lastNotifiedSeg; + + /* + * Update shared memory and discard segment boundaries that are no + * longer needed. + * + * It is safe to update shared memory before we attempt to create the + * .ready files. If our calls to XLogArchiveNotifySeg() fail, + * RemoveOldXlogFiles() will retry it as needed. + */ + if (last_notified < latest_boundary_seg - 1) + XLogCtl->lastNotifiedSeg = latest_boundary_seg - 1; + RemoveSegmentBoundariesUpTo(latest_boundary_seg); + + SpinLockRelease(&XLogCtl->segtrack_lck); + + /* + * Notify archiver about segments that are ready for archival (by + * creating the corresponding .ready files). + */ + for (XLogSegNo seg = last_notified + 1; seg < latest_boundary_seg; seg++) + XLogArchiveNotifySeg(seg, false); + + PgArchWakeup(); + } + else + SpinLockRelease(&XLogCtl->segtrack_lck); +} + /* * Remove previous backup history files. This also retries creation of * .ready files for any backup history files for which XLogArchiveNotify @@ -5230,9 +5428,17 @@ XLOGShmemInit(void) SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); + SpinLockInit(&XLogCtl->segtrack_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + XLogCtl->lastNotifiedSeg = MaxXLogSegNo; + XLogCtl->earliestSegBoundary = MaxXLogSegNo; + XLogCtl->earliestSegBoundaryRecPtr = InvalidXLogRecPtr; + XLogCtl->latestSegBoundary = MaxXLogSegNo; + XLogCtl->latestSegBoundaryRecPtr = InvalidXLogRecPtr; } /* @@ -7873,6 +8079,20 @@ StartupXLOG(void) XLogCtl->LogwrtRqst.Write = EndOfLog; XLogCtl->LogwrtRqst.Flush = EndOfLog; + /* + * Initialize XLogCtl->lastNotifiedSeg to the previous WAL file. + */ + if (XLogArchivingActive()) + { + XLogSegNo EndOfLogSeg; + + XLByteToSeg(EndOfLog, EndOfLogSeg, wal_segment_size); + + SpinLockAcquire(&XLogCtl->segtrack_lck); + XLogCtl->lastNotifiedSeg = EndOfLogSeg - 1; + SpinLockRelease(&XLogCtl->segtrack_lck); + } + /* * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE * record before resource manager writes cleanup WAL records or checkpoint @@ -8000,7 +8220,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..b9c19b2085 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -462,9 +462,12 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * by the archiver, e.g. we write 0000000100000001000000C6.ready * and the archiver then knows to archive XLOGDIR/0000000100000001000000C6, * then when complete, rename it to 0000000100000001000000C6.done + * + * Optionally, nudge the archiver process so that it'll notice the file we + * create. */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool nudge) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +492,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, let archiver know it's got work to do */ + if (nudge) PgArchWakeup(); } @@ -498,12 +501,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool nudge) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, nudge); } /* @@ -608,7 +611,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 626fae8454..6a1e16edc2 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -248,6 +248,13 @@ WalWriterMain(void) /* Process any signals received recently */ HandleWalWriterInterrupts(); + /* + * Notify the archiver of any WAL segments that are ready. We do this + * here to handle a race condition where WAL is flushed to disk prior + * to registering the segment boundary. + */ + NotifySegmentsReadyForArchive(GetFlushRecPtr()); + /* * Do what we're here for; then, if XLogBackgroundFlush() found useful * work to do, reset hibernation counter. diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 0a8ede700d..6b6ae81c2d 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -315,6 +315,7 @@ extern XLogRecPtr GetInsertRecPtr(void); extern XLogRecPtr GetFlushRecPtr(void); extern XLogRecPtr GetLastImportantRecPtr(void); extern void RemovePromoteSignalFiles(void); +extern void NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr); extern bool PromoteIsTriggered(void); extern bool CheckPromoteSignal(void); diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..935b4cb02d 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool nudge); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool nudge); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..9b455e88e3 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -46,6 +46,7 @@ typedef uint64 XLogRecPtr; * XLogSegNo - physical log file sequence number. */ typedef uint64 XLogSegNo; +#define MaxXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF) /* * TimeLineID (TLI) - identifies different database histories to prevent -- 2.30.2 --3l72er6sf3zeysw4-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v15] Avoid creating archive status ".ready" files too early @ 2021-08-23 13:06 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Alvaro Herrera @ 2021-08-23 13:06 UTC (permalink / raw) WAL records may span multiple segments, but XLogWrite() does not wait for the entire record to be written out to disk before creating archive status files. Instead, as soon as the last WAL page of the segment is written, the archive status file is created, and the archiver may process it. If PostgreSQL crashes before it is able to write and flush the rest of the record (in the next WAL segment), the wrong version of the first segment file lingers in the archive, which causes operations such as point-in-time restores to fail. To fix this, keep track of records that span across segments and ensure that segments are only marked ready-for-archival once such records have been completely written to disk. This has always been wrong, so backpatch all the way back. Author: Nathan Bossart <[email protected]> Reviewed-by: Kyotaro Horiguchi <[email protected]> Reviewed-by: Ryo Matsumura <[email protected]> Reviewed-by: Andrey Borodin <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/transam/timeline.c | 2 +- src/backend/access/transam/xlog.c | 220 +++++++++++++++++++++-- src/backend/access/transam/xlogarchive.c | 17 +- src/backend/postmaster/walwriter.c | 7 + src/backend/replication/walreceiver.c | 6 +- src/include/access/xlog.h | 1 + src/include/access/xlogarchive.h | 4 +- src/include/access/xlogdefs.h | 1 + 8 files changed, 234 insertions(+), 24 deletions(-) diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c index 8d0903c175..acd5c2431d 100644 --- a/src/backend/access/transam/timeline.c +++ b/src/backend/access/transam/timeline.c @@ -452,7 +452,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, if (XLogArchivingActive()) { TLHistoryFileName(histfname, newTLI); - XLogArchiveNotify(histfname); + XLogArchiveNotify(histfname, true); } } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e51a7a749d..8e7c3a364a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -724,6 +724,18 @@ typedef struct XLogCtlData XLogRecPtr lastFpwDisableRecPtr; slock_t info_lck; /* locks shared variables shown above */ + + /* + * Variables used to track cross-segment records. Protected by + * segtrack_lck. + */ + XLogSegNo lastNotifiedSeg; + XLogSegNo earliestSegBoundary; + XLogRecPtr earliestSegBoundaryRecPtr; + XLogSegNo latestSegBoundary; + XLogRecPtr latestSegBoundaryRecPtr; + + slock_t segtrack_lck; /* locks shared variables shown above */ } XLogCtlData; static XLogCtlData *XLogCtl = NULL; @@ -920,6 +932,7 @@ static void RemoveXlogFile(const char *segname, XLogSegNo recycleSegNo, XLogSegNo *endlogSegNo); static void UpdateLastRemovedPtr(char *filename); static void ValidateXLOGDirectoryStructure(void); +static void RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos); static void CleanupBackupHistory(void); static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force); static XLogRecord *ReadRecord(XLogReaderState *xlogreader, @@ -1154,23 +1167,56 @@ XLogInsertRecord(XLogRecData *rdata, END_CRIT_SECTION(); /* - * Update shared LogwrtRqst.Write, if we crossed page boundary. + * If we crossed page boundary, update LogwrtRqst.Write; if we crossed + * segment boundary, register that and wake up walwriter. */ if (StartPos / XLOG_BLCKSZ != EndPos / XLOG_BLCKSZ) { + XLogSegNo StartSeg; + XLogSegNo EndSeg; + + XLByteToSeg(StartPos, StartSeg, wal_segment_size); + XLByteToSeg(EndPos, EndSeg, wal_segment_size); + + /* + * Register our crossing the segment boundary if that occurred. + * + * Note that we did not use XLByteToPrevSeg() for determining the + * ending segment. This is so that a record that fits perfectly into + * the end of the segment is marked ready for archival as soon as the + * flushed pointer jumps to the next segment. + */ + if (StartSeg != EndSeg && XLogArchivingActive()) + RegisterSegmentBoundary(EndSeg, EndPos); + + /* + * Advance LogwrtRqst.Write so that it includes new block(s). + * + * We do this after registering the segment boundary so that the + * comparison with the flushed pointer below can use the latest value + * known globally. + */ SpinLockAcquire(&XLogCtl->info_lck); - /* advance global request to include new block(s) */ if (XLogCtl->LogwrtRqst.Write < EndPos) XLogCtl->LogwrtRqst.Write = EndPos; /* update local result copy while I have the chance */ LogwrtResult = XLogCtl->LogwrtResult; SpinLockRelease(&XLogCtl->info_lck); + + /* + * There's a chance that the record was already flushed to disk and we + * missed marking segments as ready for archive. If this happens, we + * nudge the WALWriter, which will take care of notifying segments as + * needed. + */ + if (StartSeg != EndSeg && XLogArchivingActive() && + LogwrtResult.Flush >= EndPos && ProcGlobal->walwriterLatch) + SetLatch(ProcGlobal->walwriterLatch); } /* * If this was an XLOG_SWITCH record, flush the record and the empty - * padding space that fills the rest of the segment, and perform - * end-of-segment actions (eg, notifying archiver). + * padding space that fills the rest of the segment. */ if (isLogSwitch) { @@ -2421,6 +2467,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) /* We should always be inside a critical section here */ Assert(CritSectionCount > 0); + Assert(LWLockHeldByMe(WALWriteLock)); /* * Update local LogwrtResult (caller probably did this already, but...) @@ -2586,11 +2633,12 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) * later. Doing it here ensures that one and only one backend will * perform this fsync. * - * This is also the right place to notify the Archiver that the - * segment is ready to copy to archival storage, and to update the - * timer for archive_timeout, and to signal for a checkpoint if - * too many logfile segments have been used since the last - * checkpoint. + * If WAL archiving is active, we attempt to notify the archiver + * of any segments that are now ready for archival. + * + * This is also the right place to update the timer for + * archive_timeout and to signal for a checkpoint if too many + * logfile segments have been used since the last checkpoint. */ if (finishing_seg) { @@ -2602,7 +2650,7 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) LogwrtResult.Flush = LogwrtResult.Write; /* end of page */ if (XLogArchivingActive()) - XLogArchiveNotifySeg(openLogSegNo); + NotifySegmentsReadyForArchive(LogwrtResult.Flush); XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL); XLogCtl->lastSegSwitchLSN = LogwrtResult.Flush; @@ -2690,6 +2738,9 @@ XLogWrite(XLogwrtRqst WriteRqst, bool flexible) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); } + + if (XLogArchivingActive()) + NotifySegmentsReadyForArchive(LogwrtResult.Flush); } /* @@ -4328,6 +4379,131 @@ ValidateXLOGDirectoryStructure(void) } } +/* + * RegisterSegmentBoundary + * + * WAL records that are split across a segment boundary require special + * treatment for archiving: the initial segment must not be archived until + * the end segment has been flushed, in case we crash before we have + * the chance to flush the end segment (because after recovery we would + * overwrite that WAL record with a different one, and so the file we + * archived no longer represents truth.) This also applies to streaming + * physical replication. + * + * To handle this, we keep track of the LSN of WAL records that cross + * segment boundaries. Two such are sufficient: the earliest and the + * latest we know about, since the flush position advances monotonically. + * WAL record writers register boundary-crossing records here, which is + * used by .ready file creation to delay until the end segment is known + * flushed. + */ +static void +RegisterSegmentBoundary(XLogSegNo seg, XLogRecPtr pos) +{ + XLogSegNo segno PG_USED_FOR_ASSERTS_ONLY; + + /* verify caller computed segment number correctly */ + AssertArg((XLByteToSeg(pos, segno, wal_segment_size), segno == seg)); + + SpinLockAcquire(&XLogCtl->segtrack_lck); + + /* + * If no segment boundaries are registered, store the new segment boundary + * in earliestSegBoundary. Otherwise, store the greater segment boundaries in + * latestSegBoundary. + */ + if (XLogCtl->earliestSegBoundary == MaxXLogSegNo) + { + XLogCtl->earliestSegBoundary = seg; + XLogCtl->earliestSegBoundaryRecPtr = pos; + } + else if (seg > XLogCtl->earliestSegBoundary && + (XLogCtl->latestSegBoundary == MaxXLogSegNo || + seg > XLogCtl->latestSegBoundary)) + { + XLogCtl->latestSegBoundary = seg; + XLogCtl->latestSegBoundaryRecPtr = pos; + } + + SpinLockRelease(&XLogCtl->segtrack_lck); +} + +/* + * NotifySegmentsReadyForArchive + * + * Mark segments as ready for archival, given that it is safe to do so. + * This function is idempotent. + */ +void +NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr) +{ + XLogSegNo latest_boundary_seg; + XLogSegNo last_notified; + XLogSegNo flushed_seg; + XLogSegNo seg; + bool keep_latest; + + XLByteToSeg(flushRecPtr, flushed_seg, wal_segment_size); + + SpinLockAcquire(&XLogCtl->segtrack_lck); + + if (XLogCtl->latestSegBoundary <= flushed_seg && + XLogCtl->latestSegBoundaryRecPtr <= flushRecPtr) + { + latest_boundary_seg = XLogCtl->latestSegBoundary; + keep_latest = false; + } + else if (XLogCtl->earliestSegBoundary <= flushed_seg && + XLogCtl->earliestSegBoundaryRecPtr <= flushRecPtr) + { + latest_boundary_seg = XLogCtl->earliestSegBoundary; + keep_latest = true; + } + else + { + SpinLockRelease(&XLogCtl->segtrack_lck); + return; + } + + last_notified = XLogCtl->lastNotifiedSeg; + + /* + * Update shared memory and discard segment boundaries that are no + * longer needed. + * + * It is safe to update shared memory before we attempt to create the + * .ready files. If our calls to XLogArchiveNotifySeg() fail, + * RemoveOldXlogFiles() will retry it as needed. + */ + if (last_notified < latest_boundary_seg - 1) + XLogCtl->lastNotifiedSeg = latest_boundary_seg - 1; + + if (keep_latest) + { + XLogCtl->earliestSegBoundary = XLogCtl->latestSegBoundary; + XLogCtl->earliestSegBoundaryRecPtr = XLogCtl->latestSegBoundaryRecPtr; + } + else + { + XLogCtl->earliestSegBoundary = MaxXLogSegNo; + XLogCtl->earliestSegBoundaryRecPtr = InvalidXLogRecPtr; + } + + XLogCtl->latestSegBoundary = MaxXLogSegNo; + XLogCtl->latestSegBoundaryRecPtr = InvalidXLogRecPtr; + + SpinLockRelease(&XLogCtl->segtrack_lck); + + /* + * Notify archiver about segments that are ready for archival (by + * creating the corresponding .ready files). + */ + for (seg = last_notified + 1; seg < latest_boundary_seg; seg++) + XLogArchiveNotifySeg(seg, false); + + PgArchWakeup(); +} + /* * Remove previous backup history files. This also retries creation of * .ready files for any backup history files for which XLogArchiveNotify @@ -5230,9 +5406,17 @@ XLOGShmemInit(void) SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); + SpinLockInit(&XLogCtl->segtrack_lck); SpinLockInit(&XLogCtl->ulsn_lck); InitSharedLatch(&XLogCtl->recoveryWakeupLatch); ConditionVariableInit(&XLogCtl->recoveryNotPausedCV); + + /* Initialize stuff for marking segments as ready for archival. */ + XLogCtl->lastNotifiedSeg = MaxXLogSegNo; + XLogCtl->earliestSegBoundary = MaxXLogSegNo; + XLogCtl->earliestSegBoundaryRecPtr = InvalidXLogRecPtr; + XLogCtl->latestSegBoundary = MaxXLogSegNo; + XLogCtl->latestSegBoundaryRecPtr = InvalidXLogRecPtr; } /* @@ -7873,6 +8057,20 @@ StartupXLOG(void) XLogCtl->LogwrtRqst.Write = EndOfLog; XLogCtl->LogwrtRqst.Flush = EndOfLog; + /* + * Initialize XLogCtl->lastNotifiedSeg to the previous WAL file. + */ + if (XLogArchivingActive()) + { + XLogSegNo EndOfLogSeg; + + XLByteToSeg(EndOfLog, EndOfLogSeg, wal_segment_size); + + SpinLockAcquire(&XLogCtl->segtrack_lck); + XLogCtl->lastNotifiedSeg = EndOfLogSeg - 1; + SpinLockRelease(&XLogCtl->segtrack_lck); + } + /* * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE * record before resource manager writes cleanup WAL records or checkpoint @@ -8000,7 +8198,7 @@ StartupXLOG(void) XLogArchiveCleanup(partialfname); durable_rename(origpath, partialpath, ERROR); - XLogArchiveNotify(partialfname); + XLogArchiveNotify(partialfname, true); } } } diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 26b023e754..b9c19b2085 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -433,7 +433,7 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); /* * If the existing file was replaced, since walsenders might have it open, @@ -462,9 +462,12 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname) * by the archiver, e.g. we write 0000000100000001000000C6.ready * and the archiver then knows to archive XLOGDIR/0000000100000001000000C6, * then when complete, rename it to 0000000100000001000000C6.done + * + * Optionally, nudge the archiver process so that it'll notice the file we + * create. */ void -XLogArchiveNotify(const char *xlog) +XLogArchiveNotify(const char *xlog, bool nudge) { char archiveStatusPath[MAXPGPATH]; FILE *fd; @@ -489,8 +492,8 @@ XLogArchiveNotify(const char *xlog) return; } - /* Notify archiver that it's got something to do */ - if (IsUnderPostmaster) + /* If caller requested, let archiver know it's got work to do */ + if (nudge) PgArchWakeup(); } @@ -498,12 +501,12 @@ XLogArchiveNotify(const char *xlog) * Convenience routine to notify using segment number representation of filename */ void -XLogArchiveNotifySeg(XLogSegNo segno) +XLogArchiveNotifySeg(XLogSegNo segno, bool nudge) { char xlog[MAXFNAMELEN]; XLogFileName(xlog, ThisTimeLineID, segno, wal_segment_size); - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, nudge); } /* @@ -608,7 +611,7 @@ XLogArchiveCheckDone(const char *xlog) return true; /* Retry creation of the .ready file */ - XLogArchiveNotify(xlog); + XLogArchiveNotify(xlog, true); return false; } diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 626fae8454..6a1e16edc2 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -248,6 +248,13 @@ WalWriterMain(void) /* Process any signals received recently */ HandleWalWriterInterrupts(); + /* + * Notify the archiver of any WAL segments that are ready. We do this + * here to handle a race condition where WAL is flushed to disk prior + * to registering the segment boundary. + */ + NotifySegmentsReadyForArchive(GetFlushRecPtr()); + /* * Do what we're here for; then, if XLogBackgroundFlush() found useful * work to do, reset hibernation counter. diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 9a2bc37fd7..60de3be92c 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -622,7 +622,7 @@ WalReceiverMain(void) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; @@ -760,7 +760,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(fname); else - XLogArchiveNotify(fname); + XLogArchiveNotify(fname, true); pfree(fname); pfree(content); @@ -915,7 +915,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS) XLogArchiveForceDone(xlogfname); else - XLogArchiveNotify(xlogfname); + XLogArchiveNotify(xlogfname, true); } recvFile = -1; diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 0a8ede700d..6b6ae81c2d 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -315,6 +315,7 @@ extern XLogRecPtr GetInsertRecPtr(void); extern XLogRecPtr GetFlushRecPtr(void); extern XLogRecPtr GetLastImportantRecPtr(void); extern void RemovePromoteSignalFiles(void); +extern void NotifySegmentsReadyForArchive(XLogRecPtr flushRecPtr); extern bool PromoteIsTriggered(void); extern bool CheckPromoteSignal(void); diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 3edd1a976c..935b4cb02d 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -23,8 +23,8 @@ extern bool RestoreArchivedFile(char *path, const char *xlogfname, extern void ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal); extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname); -extern void XLogArchiveNotify(const char *xlog); -extern void XLogArchiveNotifySeg(XLogSegNo segno); +extern void XLogArchiveNotify(const char *xlog, bool nudge); +extern void XLogArchiveNotifySeg(XLogSegNo segno, bool nudge); extern void XLogArchiveForceDone(const char *xlog); extern bool XLogArchiveCheckDone(const char *xlog); extern bool XLogArchiveIsBusy(const char *xlog); diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h index 60348d1850..9b455e88e3 100644 --- a/src/include/access/xlogdefs.h +++ b/src/include/access/xlogdefs.h @@ -46,6 +46,7 @@ typedef uint64 XLogRecPtr; * XLogSegNo - physical log file sequence number. */ typedef uint64 XLogSegNo; +#define MaxXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF) /* * TimeLineID (TLI) - identifies different database histories to prevent -- 2.30.2 --erxcxkqg4k65gkj3-- ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-18 18:07 Tom Lane <[email protected]> 0 siblings, 2 replies; 23+ messages in thread From: Tom Lane @ 2022-04-18 18:07 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Noah Misch <[email protected]>; Andres Freund <[email protected]>; Michael Paquier <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> Andrew Dunstan <[email protected]> writes: > No, I think we could probably just port the whole of src/test/PostreSQL > back if required, and have it live alongside the old modules. Each TAP > test is a separate miracle - see comments elsewhere about port > assignment in parallel TAP tests. > But that would mean we have some tests in the old flavor and some in the > new flavor in the back branches, which might get confusing. That works for back-patching entire new test scripts, but not for adding some cases to an existing script, which I think is more common. regards, tom lane ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-18 19:29 Andrew Dunstan <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 0 replies; 23+ messages in thread From: Andrew Dunstan @ 2022-04-18 19:29 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Noah Misch <[email protected]>; Andres Freund <[email protected]>; Michael Paquier <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-18 Mo 14:07, Tom Lane wrote: > Andrew Dunstan <[email protected]> writes: >> No, I think we could probably just port the whole of src/test/PostreSQL >> back if required, and have it live alongside the old modules. Each TAP >> test is a separate miracle - see comments elsewhere about port >> assignment in parallel TAP tests. >> But that would mean we have some tests in the old flavor and some in the >> new flavor in the back branches, which might get confusing. > That works for back-patching entire new test scripts, but not for adding > some cases to an existing script, which I think is more common. > > I think the only thing that should trip people up in those cases is the the new/get_new_node thing. That's complicated by the fact that the old PostgresNode module has both new() and get_new_node(), although it advises people not to use its new(). Probably the best way around that is a) rename it's new() and deal with any callers, and b) add a new new(), which would be a wrapper around get_new_node(). I'll have a play with that. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-19 15:36 Andrew Dunstan <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 2 replies; 23+ messages in thread From: Andrew Dunstan @ 2022-04-19 15:36 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Noah Misch <[email protected]>; Andres Freund <[email protected]>; Michael Paquier <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-18 Mo 14:07, Tom Lane wrote: > Andrew Dunstan <[email protected]> writes: >> No, I think we could probably just port the whole of src/test/PostreSQL >> back if required, and have it live alongside the old modules. Each TAP >> test is a separate miracle - see comments elsewhere about port >> assignment in parallel TAP tests. >> But that would mean we have some tests in the old flavor and some in the >> new flavor in the back branches, which might get confusing. > That works for back-patching entire new test scripts, but not for adding > some cases to an existing script, which I think is more common. > > I think I've come up with a better scheme that I hope will fix all or almost all of the pain complained of in this thread. I should note that we deliberately delayed making these changes until fairly early in the release 15 development cycle, and that was clearly a good decision. The attached three patches basically implement the new naming scheme for the back branches without doing away with the old scheme or doing a wholesale copy of the new modules. The first simply implements a proper "new" constructor for PostgresNode, just like we have in PostgreSQL:Test::Cluster. It's not really essential but it seems like a good idea. The second adds all the visible functionality of the PostgresNode and TestLib modules to the PostgreSQL::Test::Cluster and PostgreSQL::Test::Utils namespaces.. The third adds dummy packages so that any code doing 'use PostgreSQL::Test::Utils;' or 'use PostgreSQL::Test::Cluster;' will actually import the old modules. This last piece is where there might be some extra work needed, to export the names so that using an unqualified function or variable, say, 'slurp_file("foo");' will work. But in general, modulo that issue, I believe things should Just Work (tm). You should basically just be able to backpatch any new or modified TAP test without difficulty, sed script usage, etc. Comments welcome. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com Attachments: [text/x-patch] PostgresNode-new-method.patch (1.1K, ../../[email protected]/2-PostgresNode-new-method.patch) download | inline diff: diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 92d9303e23..94b39341ce 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -137,16 +137,22 @@ INIT =over -=item PostgresNode::new($class, $name, $pghost, $pgport) +=item PostgresNode->new(node_name, %params) -Create a new PostgresNode instance. Does not initdb or start it. - -You should generally prefer to use get_new_node() instead since it takes care -of finding port numbers, registering instances for cleanup, etc. +Class oriented alias for get_new_node() =cut sub new +{ + my $class = $_[0]; + $class->isa(__PACKAGE__) || die "new() not called as a class method"; + return get_new_node(@_); +} + +# internal subroutine used by get_new_node(). SHould not be called by any +# external client of the module. +sub _new { my ($class, $name, $pghost, $pgport) = @_; my $testname = basename($0); @@ -1229,7 +1235,7 @@ sub get_new_node } # Lock port number found by creating a new node - my $node = $class->new($name, $host, $port); + my $node = _new($class, $name, $host, $port); if ($params{install_path}) { [text/x-patch] perl-new-namespace-backport.patch (2.3K, ../../[email protected]/3-perl-new-namespace-backport.patch) download | inline diff: diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 92d9303e23..f5d2ba72e6 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -2761,4 +2761,16 @@ sub corrupt_page_checksum =cut +# support release 15+ perl module namespace + +package PostgreSQL::Test::Cluster; + +sub new +{ + shift; # remove class param from args + return PostgresNode->get_new_node(@_); +} + +sub get_free_port { return PostgresNode::get_free_port(); } + 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 0dfc414b07..92583f84b4 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -948,4 +948,46 @@ sub command_checks_all =cut +# support release 15+ perl module namespace + +package PostgreSQL::Test::Utils; + +# we don't want to export anything, but we want to support things called +# via this package name explicitly. + +# use typeglobs to alias these functions and variables + +*generate_ascii_string = *TestLib::generate_ascii_string; +*slurp_dir = *TestLib::slurp_dir; +*slurp_file = *TestLib::slurp_file; +*append_to_file = *TestLib::append_to_file; +*check_mode_recursive = *TestLib::check_mode_recursive; +*chmod_recursive = *TestLib::chmod_recursive; +*check_pg_config = *TestLib::check_pg_config; +*dir_symlink = *TestLib::dir_symlink; +*system_or_bail = *TestLib::system_or_bail; +*system_log = *TestLib::system_log; +*run_log = *TestLib::run_log; +*run_command = *TestLib::run_command; +sub pump_until { die "pump_until not implemented in TestLib"; } +*command_ok = *TestLib::command_ok; +*command_fails = *TestLib::command_fails; +*command_exit_is = *TestLib::command_exit_is; +*program_help_ok = *TestLib::program_help_ok; +*program_version_ok = *TestLib::program_version_ok; +*program_options_handling_ok = *TestLib::program_options_handling_ok; +*command_like = *TestLib::command_like; +*command_like_safe = *TestLib::command_like_safe; +*command_fails_like = *TestLib::command_fails_like; +*command_checks_all = *TestLib::command_checks_all; + +*windows_os = *TestLib::windows_os; +*is_msys2 = *TestLib::is_msys2; +*use_unix_sockets = *TestLib::use_unix_sockets; +*timeout_default = *TestLib::timeout_default; +*tmp_check = *TestLib::tmp_check; +*log_path = *TestLib::log_path; +*test_logfile = *TestLib::test_log_file; + + 1; [text/x-patch] perl-new-namespace-backport-2.patch (1.0K, ../../[email protected]/4-perl-new-namespace-backport-2.patch) download | inline diff: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm new file mode 100644 index 0000000000..1e2fb50d6d --- /dev/null +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -0,0 +1,16 @@ + +# Copyright (c) 2022, PostgreSQL Global Development Group + +# allow use of release 15+ perl namespace in older branches +# just 'use' the older module name. +# See PostgresNode.pm for function implementations + +package PostgreSQL::Test::Cluster; + +use strict; +use warnings; + +use PostgresNode; + +1; + diff --git a/src/test/perl/PostgreSQL/Test/Utils.pl b/src/test/perl/PostgreSQL/Test/Utils.pl new file mode 100644 index 0000000000..752ce38a74 --- /dev/null +++ b/src/test/perl/PostgreSQL/Test/Utils.pl @@ -0,0 +1,14 @@ +# Copyright (c) 2022, PostgreSQL Global Development Group + +# allow use of release 15+ perl namespace in older branches +# just 'use' the older module name. +# See TestLib.pm for alias assignment + +package PostgreSQL::Test::Utils; + +use strict; +use warnings; + +use TestLib; + +1; ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-19 17:15 Andres Freund <[email protected]> parent: Andrew Dunstan <[email protected]> 1 sibling, 0 replies; 23+ messages in thread From: Andres Freund @ 2022-04-19 17:15 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> Hi, On 2022-04-19 11:36:44 -0400, Andrew Dunstan wrote: > The attached three patches basically implement the new naming scheme for > the back branches without doing away with the old scheme or doing a > wholesale copy of the new modules. That sounds like good plan! I don't know perl enough to comment on the details, but it looks roughly sane to me. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-19 20:06 Andrew Dunstan <[email protected]> parent: Andrew Dunstan <[email protected]> 1 sibling, 1 reply; 23+ messages in thread From: Andrew Dunstan @ 2022-04-19 20:06 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Noah Misch <[email protected]>; Andres Freund <[email protected]>; Michael Paquier <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-19 Tu 11:36, Andrew Dunstan wrote: > On 2022-04-18 Mo 14:07, Tom Lane wrote: >> Andrew Dunstan <[email protected]> writes: >>> No, I think we could probably just port the whole of src/test/PostreSQL >>> back if required, and have it live alongside the old modules. Each TAP >>> test is a separate miracle - see comments elsewhere about port >>> assignment in parallel TAP tests. >>> But that would mean we have some tests in the old flavor and some in the >>> new flavor in the back branches, which might get confusing. >> That works for back-patching entire new test scripts, but not for adding >> some cases to an existing script, which I think is more common. >> >> > > I think I've come up with a better scheme that I hope will fix all or > almost all of the pain complained of in this thread. I should note that > we deliberately delayed making these changes until fairly early in the > release 15 development cycle, and that was clearly a good decision. > > The attached three patches basically implement the new naming scheme for > the back branches without doing away with the old scheme or doing a > wholesale copy of the new modules. > > The first simply implements a proper "new" constructor for PostgresNode, > just like we have in PostgreSQL:Test::Cluster. It's not really essential > but it seems like a good idea. The second adds all the visible > functionality of the PostgresNode and TestLib modules to the > PostgreSQL::Test::Cluster and PostgreSQL::Test::Utils namespaces.. The > third adds dummy packages so that any code doing 'use > PostgreSQL::Test::Utils;' or 'use PostgreSQL::Test::Cluster;' will > actually import the old modules. This last piece is where there might be > some extra work needed, to export the names so that using an unqualified > function or variable, say, 'slurp_file("foo");' will work. But in > general, modulo that issue, I believe things should Just Work (tm). You > should basically just be able to backpatch any new or modified TAP test > without difficulty, sed script usage, etc. > > Comments welcome. > > Here's a version with a fixed third patch that corrects a file misnaming and fixes the export issue referred to above. Passes my testing so far. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com Attachments: [text/x-patch] PostgresNode-new-method.patch (1.1K, ../../[email protected]/2-PostgresNode-new-method.patch) download | inline diff: diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 92d9303e23..94b39341ce 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -137,16 +137,22 @@ INIT =over -=item PostgresNode::new($class, $name, $pghost, $pgport) +=item PostgresNode->new(node_name, %params) -Create a new PostgresNode instance. Does not initdb or start it. - -You should generally prefer to use get_new_node() instead since it takes care -of finding port numbers, registering instances for cleanup, etc. +Class oriented alias for get_new_node() =cut sub new +{ + my $class = $_[0]; + $class->isa(__PACKAGE__) || die "new() not called as a class method"; + return get_new_node(@_); +} + +# internal subroutine used by get_new_node(). SHould not be called by any +# external client of the module. +sub _new { my ($class, $name, $pghost, $pgport) = @_; my $testname = basename($0); @@ -1229,7 +1235,7 @@ sub get_new_node } # Lock port number found by creating a new node - my $node = $class->new($name, $host, $port); + my $node = _new($class, $name, $host, $port); if ($params{install_path}) { [text/x-patch] perl-new-namespace-backport.patch (2.3K, ../../[email protected]/3-perl-new-namespace-backport.patch) download | inline diff: diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 92d9303e23..f5d2ba72e6 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -2761,4 +2761,16 @@ sub corrupt_page_checksum =cut +# support release 15+ perl module namespace + +package PostgreSQL::Test::Cluster; + +sub new +{ + shift; # remove class param from args + return PostgresNode->get_new_node(@_); +} + +sub get_free_port { return PostgresNode::get_free_port(); } + 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 0dfc414b07..92583f84b4 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -948,4 +948,46 @@ sub command_checks_all =cut +# support release 15+ perl module namespace + +package PostgreSQL::Test::Utils; + +# we don't want to export anything, but we want to support things called +# via this package name explicitly. + +# use typeglobs to alias these functions and variables + +*generate_ascii_string = *TestLib::generate_ascii_string; +*slurp_dir = *TestLib::slurp_dir; +*slurp_file = *TestLib::slurp_file; +*append_to_file = *TestLib::append_to_file; +*check_mode_recursive = *TestLib::check_mode_recursive; +*chmod_recursive = *TestLib::chmod_recursive; +*check_pg_config = *TestLib::check_pg_config; +*dir_symlink = *TestLib::dir_symlink; +*system_or_bail = *TestLib::system_or_bail; +*system_log = *TestLib::system_log; +*run_log = *TestLib::run_log; +*run_command = *TestLib::run_command; +sub pump_until { die "pump_until not implemented in TestLib"; } +*command_ok = *TestLib::command_ok; +*command_fails = *TestLib::command_fails; +*command_exit_is = *TestLib::command_exit_is; +*program_help_ok = *TestLib::program_help_ok; +*program_version_ok = *TestLib::program_version_ok; +*program_options_handling_ok = *TestLib::program_options_handling_ok; +*command_like = *TestLib::command_like; +*command_like_safe = *TestLib::command_like_safe; +*command_fails_like = *TestLib::command_fails_like; +*command_checks_all = *TestLib::command_checks_all; + +*windows_os = *TestLib::windows_os; +*is_msys2 = *TestLib::is_msys2; +*use_unix_sockets = *TestLib::use_unix_sockets; +*timeout_default = *TestLib::timeout_default; +*tmp_check = *TestLib::tmp_check; +*log_path = *TestLib::log_path; +*test_logfile = *TestLib::test_log_file; + + 1; [text/x-patch] perl-new-namespace-backport-2-v2.patch (1.6K, ../../[email protected]/4-perl-new-namespace-backport-2-v2.patch) download | inline diff: diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm new file mode 100644 index 0000000000..1e2fb50d6d --- /dev/null +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -0,0 +1,16 @@ + +# Copyright (c) 2022, PostgreSQL Global Development Group + +# allow use of release 15+ perl namespace in older branches +# just 'use' the older module name. +# See PostgresNode.pm for function implementations + +package PostgreSQL::Test::Cluster; + +use strict; +use warnings; + +use PostgresNode; + +1; + diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm new file mode 100644 index 0000000000..bdbbd6e470 --- /dev/null +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -0,0 +1,48 @@ +# Copyright (c) 2022, PostgreSQL Global Development Group + +# allow use of release 15+ perl namespace in older branches +# just 'use' the older module name. +# We export the same names as the v15 module. +# See TestLib.pm for alias assignment that makes this all work. + +package PostgreSQL::Test::Utils; + +use strict; +use warnings; + +use Exporter 'import'; + +use TestLib; + +our @EXPORT = qw( + generate_ascii_string + slurp_dir + slurp_file + append_to_file + check_mode_recursive + chmod_recursive + check_pg_config + dir_symlink + system_or_bail + system_log + run_log + run_command + pump_until + + command_ok + command_fails + command_exit_is + program_help_ok + program_version_ok + program_options_handling_ok + command_like + command_like_safe + command_fails_like + command_checks_all + + $windows_os + $is_msys2 + $use_unix_sockets +); + +1; ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-19 22:39 Michael Paquier <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Michael Paquier @ 2022-04-19 22:39 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Apr 19, 2022 at 04:06:28PM -0400, Andrew Dunstan wrote: > Here's a version with a fixed third patch that corrects a file misnaming > and fixes the export issue referred to above. Passes my testing so far. Wow. That's really cool. You are combining the best of both worlds here to ease backpatching, as far as I understand what you wrote. +*generate_ascii_string = *TestLib::generate_ascii_string; +*slurp_dir = *TestLib::slurp_dir; +*slurp_file = *TestLib::slurp_file; I am not sure if it is possible and my perl-fu is limited in this area, but could a failure be enforced when loading this path if a new routine added in TestLib.pm is forgotten in this list? -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-19 23:24 Andrew Dunstan <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 2 replies; 23+ messages in thread From: Andrew Dunstan @ 2022-04-19 23:24 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-19 Tu 18:39, Michael Paquier wrote: > On Tue, Apr 19, 2022 at 04:06:28PM -0400, Andrew Dunstan wrote: >> Here's a version with a fixed third patch that corrects a file misnaming >> and fixes the export issue referred to above. Passes my testing so far. > Wow. That's really cool. You are combining the best of both worlds > here to ease backpatching, as far as I understand what you wrote. Thanks. > > +*generate_ascii_string = *TestLib::generate_ascii_string; > +*slurp_dir = *TestLib::slurp_dir; > +*slurp_file = *TestLib::slurp_file; > > I am not sure if it is possible and my perl-fu is limited in this > area, but could a failure be enforced when loading this path if a new > routine added in TestLib.pm is forgotten in this list? Not very easily that I'm aware of, but maybe some superior perl wizard will know better. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-20 00:30 Michael Paquier <[email protected]> parent: Andrew Dunstan <[email protected]> 1 sibling, 1 reply; 23+ messages in thread From: Michael Paquier @ 2022-04-20 00:30 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: > On 2022-04-19 Tu 18:39, Michael Paquier wrote: >> +*generate_ascii_string = *TestLib::generate_ascii_string; >> +*slurp_dir = *TestLib::slurp_dir; >> +*slurp_file = *TestLib::slurp_file; >> >> I am not sure if it is possible and my perl-fu is limited in this >> area, but could a failure be enforced when loading this path if a new >> routine added in TestLib.pm is forgotten in this list? > > Not very easily that I'm aware of, but maybe some superior perl wizard > will know better. Okay. Please do not consider this as a blocker. I was just wondering about ways to ease more the error reports when it comes to back-patching, and this would move the error stack a bit earlier. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-20 19:56 Andrew Dunstan <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Andrew Dunstan @ 2022-04-20 19:56 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-19 Tu 20:30, Michael Paquier wrote: > On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: >> On 2022-04-19 Tu 18:39, Michael Paquier wrote: >>> +*generate_ascii_string = *TestLib::generate_ascii_string; >>> +*slurp_dir = *TestLib::slurp_dir; >>> +*slurp_file = *TestLib::slurp_file; >>> >>> I am not sure if it is possible and my perl-fu is limited in this >>> area, but could a failure be enforced when loading this path if a new >>> routine added in TestLib.pm is forgotten in this list? >> Not very easily that I'm aware of, but maybe some superior perl wizard >> will know better. > Okay. Please do not consider this as a blocker. I was just wondering > about ways to ease more the error reports when it comes to > back-patching, and this would move the error stack a bit earlier. There are a few other things that could make backpatching harder, and while they are not related to the namespace issue they do affect a bit how that is managed. The following variables are missing in various versions of TestLib: in version 13 and earlier: $is_msys2, $timeout_default in version 12 and earlier: $use_unix_sockets and the following functions are missing: in version 14 and earlier: pump_until in version 13 and earlier: dir_symlink in version 11 and earlier: run_command in version 10: check_mode_recursive, chmod_recursive, check_pg_config (Also in version 10 command_checks_all exists but isn't exported. I'm inclined just to remedy that along the way) Turning to PostgresNode, the class-wide function get_free_port is absent from version 10, and the following instance methods are absent from some or all of the back branches: adjust_conf, clean_node, command_fails_like, config_data, connect_fails, connect_ok, corrupt_page_checksum, group_access, installed_command, install_path, interactive_psql, logrotate, set_recovery_mode, set_standby_mode, wait_for_log We don't export or provide aliases for any of these instance methods in these patches, but attempts to use them in backpatched code will fail where they are absent, so I thought it worth mentioning. Basically I propose just to remove any mention of the Testlib items and get_free_port from the export and alias lists for versions where they are absent. If backpatchers need a function they can backport it if necessary. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-21 04:11 Michael Paquier <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Michael Paquier @ 2022-04-21 04:11 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Apr 20, 2022 at 03:56:17PM -0400, Andrew Dunstan wrote: > Basically I propose just to remove any mention of the Testlib items and > get_free_port from the export and alias lists for versions where they > are absent. If backpatchers need a function they can backport it if > necessary. Agreed. I am fine to stick to that (I may have done that only once or twice in the past years, so that does not happen a lot either IMO). The patch in itself looks like an improvement in the right direction, so +1 from me. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-21 13:42 Andrew Dunstan <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Andrew Dunstan @ 2022-04-21 13:42 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Noah Misch <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-21 Th 00:11, Michael Paquier wrote: > On Wed, Apr 20, 2022 at 03:56:17PM -0400, Andrew Dunstan wrote: >> Basically I propose just to remove any mention of the Testlib items and >> get_free_port from the export and alias lists for versions where they >> are absent. If backpatchers need a function they can backport it if >> necessary. > Agreed. I am fine to stick to that (I may have done that only once or > twice in the past years, so that does not happen a lot either IMO). > The patch in itself looks like an improvement in the right direction, > so +1 from me. Thanks, pushed. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-04-22 18:36 Andres Freund <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Andres Freund @ 2022-04-22 18:36 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Noah Misch <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-04-21 09:42:44 -0400, Andrew Dunstan wrote: > On 2022-04-21 Th 00:11, Michael Paquier wrote: > > On Wed, Apr 20, 2022 at 03:56:17PM -0400, Andrew Dunstan wrote: > >> Basically I propose just to remove any mention of the Testlib items and > >> get_free_port from the export and alias lists for versions where they > >> are absent. If backpatchers need a function they can backport it if > >> necessary. > > Agreed. I am fine to stick to that (I may have done that only once or > > twice in the past years, so that does not happen a lot either IMO). > > The patch in itself looks like an improvement in the right direction, > > so +1 from me. > Thanks, pushed. Thanks for working on this! ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-06-22 07:21 Noah Misch <[email protected]> parent: Andrew Dunstan <[email protected]> 1 sibling, 1 reply; 23+ messages in thread From: Noah Misch @ 2022-06-22 07:21 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: > On 2022-04-19 Tu 18:39, Michael Paquier wrote: > > +*generate_ascii_string = *TestLib::generate_ascii_string; > > +*slurp_dir = *TestLib::slurp_dir; > > +*slurp_file = *TestLib::slurp_file; > > > > I am not sure if it is possible and my perl-fu is limited in this > > area, but could a failure be enforced when loading this path if a new > > routine added in TestLib.pm is forgotten in this list? > > Not very easily that I'm aware of, but maybe some superior perl wizard > will know better. One can alias the symbol table, like https://metacpan.org/pod/Package::Alias does. I'm attaching what I plan to use. Today, check-world fails after sed -i 's/TestLib/PostgreSQL::Test::Utils/g; s/PostgresNode/PostgreSQL::Test::Cluster/g' **/*.pl on REL_14_STABLE, because today's alias list is incomplete. With this change, the same check-world passes. Author: Noah Misch <[email protected]> Commit: Noah Misch <[email protected]> For PostgreSQL::Test compatibility, alias entire package symbol tables. Remove the need to edit back-branch-specific code sites when back-patching the addition of a PostgreSQL::Test::Utils symbol. Replace per-symbol, incomplete alias lists. Give old and new package names the same EXPORT and EXPORT_OK semantics. Back-patch to v10 (all supported versions). Reviewed by FIXME. Discussion: https://postgr.es/m/FIXME diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 12339c2..a855fbc 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -1,9 +1,9 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# See PostgresNode.pm for function implementations +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. See PostgresNode::new +# for behavior reacting to the class name. package PostgreSQL::Test::Cluster; @@ -11,5 +11,8 @@ use strict; use warnings; use PostgresNode; +BEGIN { *PostgreSQL::Test::Cluster:: = \*PostgresNode::; } + +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index bdbbd6e..e743bdf 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -1,48 +1,16 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# We export the same names as the v15 module. -# See TestLib.pm for alias assignment that makes this all work. +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. package PostgreSQL::Test::Utils; use strict; use warnings; -use Exporter 'import'; - use TestLib; +BEGIN { *PostgreSQL::Test::Utils:: = \*TestLib::; } -our @EXPORT = qw( - generate_ascii_string - slurp_dir - slurp_file - append_to_file - check_mode_recursive - chmod_recursive - check_pg_config - dir_symlink - system_or_bail - system_log - run_log - run_command - pump_until - - command_ok - command_fails - command_exit_is - program_help_ok - program_version_ok - program_options_handling_ok - command_like - command_like_safe - command_fails_like - command_checks_all - - $windows_os - $is_msys2 - $use_unix_sockets -); +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index be90963..5b8c7a9 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -149,6 +149,11 @@ of finding port numbers, registering instances for cleanup, etc. sub new { my ($class, $name, $pghost, $pgport) = @_; + + # Use release 15+ semantics when called under a release 15+ name. + return PostgresNode->get_new_node(@_[ 1 .. $#_ ]) + if $class ne 'PostgresNode'; + my $testname = basename($0); $testname =~ s/\.[^.]+$//; my $self = { @@ -2796,18 +2801,4 @@ sub corrupt_page_checksum =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Cluster; ## no critic (ProhibitMultiplePackages) - -sub new -{ - shift; # remove class param from args - return PostgresNode->get_new_node(@_); -} - -no warnings 'once'; - -*get_free_port = *PostgresNode::get_free_port; - 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index f3ee20a..610050e 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -979,46 +979,4 @@ sub command_checks_all =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Utils; ## no critic (ProhibitMultiplePackages) - -# we don't want to export anything here, but we want to support things called -# via this package name explicitly. - -# use typeglobs to alias these functions and variables - -no warnings qw(once); - -*generate_ascii_string = *TestLib::generate_ascii_string; -*slurp_dir = *TestLib::slurp_dir; -*slurp_file = *TestLib::slurp_file; -*append_to_file = *TestLib::append_to_file; -*check_mode_recursive = *TestLib::check_mode_recursive; -*chmod_recursive = *TestLib::chmod_recursive; -*check_pg_config = *TestLib::check_pg_config; -*dir_symlink = *TestLib::dir_symlink; -*system_or_bail = *TestLib::system_or_bail; -*system_log = *TestLib::system_log; -*run_log = *TestLib::run_log; -*run_command = *TestLib::run_command; -*command_ok = *TestLib::command_ok; -*command_fails = *TestLib::command_fails; -*command_exit_is = *TestLib::command_exit_is; -*program_help_ok = *TestLib::program_help_ok; -*program_version_ok = *TestLib::program_version_ok; -*program_options_handling_ok = *TestLib::program_options_handling_ok; -*command_like = *TestLib::command_like; -*command_like_safe = *TestLib::command_like_safe; -*command_fails_like = *TestLib::command_fails_like; -*command_checks_all = *TestLib::command_checks_all; - -*windows_os = *TestLib::windows_os; -*is_msys2 = *TestLib::is_msys2; -*use_unix_sockets = *TestLib::use_unix_sockets; -*timeout_default = *TestLib::timeout_default; -*tmp_check = *TestLib::tmp_check; -*log_path = *TestLib::log_path; -*test_logfile = *TestLib::test_log_file; - 1; Attachments: [text/plain] alias-perl-symbol-tables-v1.patch (5.3K, ../../[email protected]/2-alias-perl-symbol-tables-v1.patch) download | inline diff: Author: Noah Misch <[email protected]> Commit: Noah Misch <[email protected]> For PostgreSQL::Test compatibility, alias entire package symbol tables. Remove the need to edit back-branch-specific code sites when back-patching the addition of a PostgreSQL::Test::Utils symbol. Replace per-symbol, incomplete alias lists. Give old and new package names the same EXPORT and EXPORT_OK semantics. Back-patch to v10 (all supported versions). Reviewed by FIXME. Discussion: https://postgr.es/m/FIXME diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 12339c2..a855fbc 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -1,9 +1,9 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# See PostgresNode.pm for function implementations +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. See PostgresNode::new +# for behavior reacting to the class name. package PostgreSQL::Test::Cluster; @@ -11,5 +11,8 @@ use strict; use warnings; use PostgresNode; +BEGIN { *PostgreSQL::Test::Cluster:: = \*PostgresNode::; } + +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index bdbbd6e..e743bdf 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -1,48 +1,16 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# We export the same names as the v15 module. -# See TestLib.pm for alias assignment that makes this all work. +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. package PostgreSQL::Test::Utils; use strict; use warnings; -use Exporter 'import'; - use TestLib; +BEGIN { *PostgreSQL::Test::Utils:: = \*TestLib::; } -our @EXPORT = qw( - generate_ascii_string - slurp_dir - slurp_file - append_to_file - check_mode_recursive - chmod_recursive - check_pg_config - dir_symlink - system_or_bail - system_log - run_log - run_command - pump_until - - command_ok - command_fails - command_exit_is - program_help_ok - program_version_ok - program_options_handling_ok - command_like - command_like_safe - command_fails_like - command_checks_all - - $windows_os - $is_msys2 - $use_unix_sockets -); +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index be90963..5b8c7a9 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -149,6 +149,11 @@ of finding port numbers, registering instances for cleanup, etc. sub new { my ($class, $name, $pghost, $pgport) = @_; + + # Use release 15+ semantics when called under a release 15+ name. + return PostgresNode->get_new_node(@_[ 1 .. $#_ ]) + if $class ne 'PostgresNode'; + my $testname = basename($0); $testname =~ s/\.[^.]+$//; my $self = { @@ -2796,18 +2801,4 @@ sub corrupt_page_checksum =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Cluster; ## no critic (ProhibitMultiplePackages) - -sub new -{ - shift; # remove class param from args - return PostgresNode->get_new_node(@_); -} - -no warnings 'once'; - -*get_free_port = *PostgresNode::get_free_port; - 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index f3ee20a..610050e 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -979,46 +979,4 @@ sub command_checks_all =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Utils; ## no critic (ProhibitMultiplePackages) - -# we don't want to export anything here, but we want to support things called -# via this package name explicitly. - -# use typeglobs to alias these functions and variables - -no warnings qw(once); - -*generate_ascii_string = *TestLib::generate_ascii_string; -*slurp_dir = *TestLib::slurp_dir; -*slurp_file = *TestLib::slurp_file; -*append_to_file = *TestLib::append_to_file; -*check_mode_recursive = *TestLib::check_mode_recursive; -*chmod_recursive = *TestLib::chmod_recursive; -*check_pg_config = *TestLib::check_pg_config; -*dir_symlink = *TestLib::dir_symlink; -*system_or_bail = *TestLib::system_or_bail; -*system_log = *TestLib::system_log; -*run_log = *TestLib::run_log; -*run_command = *TestLib::run_command; -*command_ok = *TestLib::command_ok; -*command_fails = *TestLib::command_fails; -*command_exit_is = *TestLib::command_exit_is; -*program_help_ok = *TestLib::program_help_ok; -*program_version_ok = *TestLib::program_version_ok; -*program_options_handling_ok = *TestLib::program_options_handling_ok; -*command_like = *TestLib::command_like; -*command_like_safe = *TestLib::command_like_safe; -*command_fails_like = *TestLib::command_fails_like; -*command_checks_all = *TestLib::command_checks_all; - -*windows_os = *TestLib::windows_os; -*is_msys2 = *TestLib::is_msys2; -*use_unix_sockets = *TestLib::use_unix_sockets; -*timeout_default = *TestLib::timeout_default; -*tmp_check = *TestLib::tmp_check; -*log_path = *TestLib::log_path; -*test_logfile = *TestLib::test_log_file; - 1; ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-06-22 15:03 Andrew Dunstan <[email protected]> parent: Noah Misch <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Andrew Dunstan @ 2022-06-22 15:03 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On 2022-06-22 We 03:21, Noah Misch wrote: > On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: >> On 2022-04-19 Tu 18:39, Michael Paquier wrote: >>> +*generate_ascii_string = *TestLib::generate_ascii_string; >>> +*slurp_dir = *TestLib::slurp_dir; >>> +*slurp_file = *TestLib::slurp_file; >>> >>> I am not sure if it is possible and my perl-fu is limited in this >>> area, but could a failure be enforced when loading this path if a new >>> routine added in TestLib.pm is forgotten in this list? >> Not very easily that I'm aware of, but maybe some superior perl wizard >> will know better. > One can alias the symbol table, like https://metacpan.org/pod/Package::Alias > does. I'm attaching what I plan to use. Today, check-world fails after > > sed -i 's/TestLib/PostgreSQL::Test::Utils/g; s/PostgresNode/PostgreSQL::Test::Cluster/g' **/*.pl > > on REL_14_STABLE, because today's alias list is incomplete. With this change, > the same check-world passes. Nice. 30 years of writing perl and I'm still learning of nifty features. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-06-24 05:45 Noah Misch <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 23+ messages in thread From: Noah Misch @ 2022-06-24 05:45 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Jun 22, 2022 at 11:03:22AM -0400, Andrew Dunstan wrote: > On 2022-06-22 We 03:21, Noah Misch wrote: > > On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: > >> On 2022-04-19 Tu 18:39, Michael Paquier wrote: > >>> +*generate_ascii_string = *TestLib::generate_ascii_string; > >>> +*slurp_dir = *TestLib::slurp_dir; > >>> +*slurp_file = *TestLib::slurp_file; > >>> > >>> I am not sure if it is possible and my perl-fu is limited in this > >>> area, but could a failure be enforced when loading this path if a new > >>> routine added in TestLib.pm is forgotten in this list? > >> Not very easily that I'm aware of, but maybe some superior perl wizard > >> will know better. > > One can alias the symbol table, like https://metacpan.org/pod/Package::Alias > > does. I'm attaching what I plan to use. Today, check-world fails after > > > > sed -i 's/TestLib/PostgreSQL::Test::Utils/g; s/PostgresNode/PostgreSQL::Test::Cluster/g' **/*.pl > > > > on REL_14_STABLE, because today's alias list is incomplete. With this change, > > the same check-world passes. The patch wasn't sufficient to make that experiment pass for REL_10_STABLE, where 017_shm.pl uses the %params argument of get_new_node(). The problem call stack had PostgreSQL::Test::Cluster->get_new_code calling PostgreSQL::Test::Cluster->new, which needs v14- semantics. Here's a fixed version, just changing the new() hack. I suspect v1 also misbehaved for non-core tests that subclass PostgresNode (via the approach from commit 54dacc7) or PostgreSQL::Test::Cluster. I expect this version will work with subclasses written for v14- and with subclasses written for v15+. I didn't actually write dummy subclasses to test, and the relevant permutations are numerous (e.g. whether or not the subclass overrides new(), whether or not the subclass overrides get_new_node()). > Nice. 30 years of writing perl and I'm still learning of nifty features. Thanks for reviewing. commit 5155e0f Author: Noah Misch <[email protected]> AuthorDate: Thu Jun 23 15:31:41 2022 -0700 Commit: Noah Misch <[email protected]> CommitDate: Thu Jun 23 15:31:41 2022 -0700 For PostgreSQL::Test compatibility, alias entire package symbol tables. Remove the need to edit back-branch-specific code sites when back-patching the addition of a PostgreSQL::Test::Utils symbol. Replace per-symbol, incomplete alias lists. Give old and new package names the same EXPORT and EXPORT_OK semantics. Back-patch to v10 (all supported versions). Reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/[email protected] --- src/test/perl/PostgreSQL/Test/Cluster.pm | 9 ++++--- src/test/perl/PostgreSQL/Test/Utils.pm | 40 +++--------------------------- src/test/perl/PostgresNode.pm | 23 +++++++---------- src/test/perl/TestLib.pm | 42 -------------------------------- 4 files changed, 19 insertions(+), 95 deletions(-) diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 12339c2..a855fbc 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -1,9 +1,9 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# See PostgresNode.pm for function implementations +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. See PostgresNode::new +# for behavior reacting to the class name. package PostgreSQL::Test::Cluster; @@ -11,5 +11,8 @@ use strict; use warnings; use PostgresNode; +BEGIN { *PostgreSQL::Test::Cluster:: = \*PostgresNode::; } + +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index bdbbd6e..e743bdf 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -1,48 +1,16 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# We export the same names as the v15 module. -# See TestLib.pm for alias assignment that makes this all work. +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. package PostgreSQL::Test::Utils; use strict; use warnings; -use Exporter 'import'; - use TestLib; +BEGIN { *PostgreSQL::Test::Utils:: = \*TestLib::; } -our @EXPORT = qw( - generate_ascii_string - slurp_dir - slurp_file - append_to_file - check_mode_recursive - chmod_recursive - check_pg_config - dir_symlink - system_or_bail - system_log - run_log - run_command - pump_until - - command_ok - command_fails - command_exit_is - program_help_ok - program_version_ok - program_options_handling_ok - command_like - command_like_safe - command_fails_like - command_checks_all - - $windows_os - $is_msys2 - $use_unix_sockets -); +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index be90963..41bb582 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -149,6 +149,15 @@ of finding port numbers, registering instances for cleanup, etc. sub new { my ($class, $name, $pghost, $pgport) = @_; + + # Use release 15+ semantics when the arguments look like (node_name, + # %params). We can't use $class to decide, because get_new_node() passes + # a v14- argument list regardless of the class. $class might be an + # out-of-core subclass. $class->isa('PostgresNode') returns true even for + # descendants of PostgreSQL::Test::Cluster, so it doesn't help. + return $class->get_new_node(@_[ 1 .. $#_ ]) + if !$pghost or !$pgport or $pghost =~ /^[a-zA-Z0-9_]$/; + my $testname = basename($0); $testname =~ s/\.[^.]+$//; my $self = { @@ -2796,18 +2805,4 @@ sub corrupt_page_checksum =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Cluster; ## no critic (ProhibitMultiplePackages) - -sub new -{ - shift; # remove class param from args - return PostgresNode->get_new_node(@_); -} - -no warnings 'once'; - -*get_free_port = *PostgresNode::get_free_port; - 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index f3ee20a..610050e 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -979,46 +979,4 @@ sub command_checks_all =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Utils; ## no critic (ProhibitMultiplePackages) - -# we don't want to export anything here, but we want to support things called -# via this package name explicitly. - -# use typeglobs to alias these functions and variables - -no warnings qw(once); - -*generate_ascii_string = *TestLib::generate_ascii_string; -*slurp_dir = *TestLib::slurp_dir; -*slurp_file = *TestLib::slurp_file; -*append_to_file = *TestLib::append_to_file; -*check_mode_recursive = *TestLib::check_mode_recursive; -*chmod_recursive = *TestLib::chmod_recursive; -*check_pg_config = *TestLib::check_pg_config; -*dir_symlink = *TestLib::dir_symlink; -*system_or_bail = *TestLib::system_or_bail; -*system_log = *TestLib::system_log; -*run_log = *TestLib::run_log; -*run_command = *TestLib::run_command; -*command_ok = *TestLib::command_ok; -*command_fails = *TestLib::command_fails; -*command_exit_is = *TestLib::command_exit_is; -*program_help_ok = *TestLib::program_help_ok; -*program_version_ok = *TestLib::program_version_ok; -*program_options_handling_ok = *TestLib::program_options_handling_ok; -*command_like = *TestLib::command_like; -*command_like_safe = *TestLib::command_like_safe; -*command_fails_like = *TestLib::command_fails_like; -*command_checks_all = *TestLib::command_checks_all; - -*windows_os = *TestLib::windows_os; -*is_msys2 = *TestLib::is_msys2; -*use_unix_sockets = *TestLib::use_unix_sockets; -*timeout_default = *TestLib::timeout_default; -*tmp_check = *TestLib::tmp_check; -*log_path = *TestLib::log_path; -*test_logfile = *TestLib::test_log_file; - 1; Attachments: [text/plain] alias-perl-symbol-tables-v2.patch (6.1K, ../../[email protected]/2-alias-perl-symbol-tables-v2.patch) download | inline diff: commit 5155e0f Author: Noah Misch <[email protected]> AuthorDate: Thu Jun 23 15:31:41 2022 -0700 Commit: Noah Misch <[email protected]> CommitDate: Thu Jun 23 15:31:41 2022 -0700 For PostgreSQL::Test compatibility, alias entire package symbol tables. Remove the need to edit back-branch-specific code sites when back-patching the addition of a PostgreSQL::Test::Utils symbol. Replace per-symbol, incomplete alias lists. Give old and new package names the same EXPORT and EXPORT_OK semantics. Back-patch to v10 (all supported versions). Reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/[email protected] --- src/test/perl/PostgreSQL/Test/Cluster.pm | 9 ++++--- src/test/perl/PostgreSQL/Test/Utils.pm | 40 +++--------------------------- src/test/perl/PostgresNode.pm | 23 +++++++---------- src/test/perl/TestLib.pm | 42 -------------------------------- 4 files changed, 19 insertions(+), 95 deletions(-) diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 12339c2..a855fbc 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -1,9 +1,9 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# See PostgresNode.pm for function implementations +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. See PostgresNode::new +# for behavior reacting to the class name. package PostgreSQL::Test::Cluster; @@ -11,5 +11,8 @@ use strict; use warnings; use PostgresNode; +BEGIN { *PostgreSQL::Test::Cluster:: = \*PostgresNode::; } + +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index bdbbd6e..e743bdf 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -1,48 +1,16 @@ # Copyright (c) 2022, PostgreSQL Global Development Group -# allow use of release 15+ perl namespace in older branches -# just 'use' the older module name. -# We export the same names as the v15 module. -# See TestLib.pm for alias assignment that makes this all work. +# Allow use of release 15+ Perl package name in older branches, by giving that +# package the same symbol table as the older package. package PostgreSQL::Test::Utils; use strict; use warnings; -use Exporter 'import'; - use TestLib; +BEGIN { *PostgreSQL::Test::Utils:: = \*TestLib::; } -our @EXPORT = qw( - generate_ascii_string - slurp_dir - slurp_file - append_to_file - check_mode_recursive - chmod_recursive - check_pg_config - dir_symlink - system_or_bail - system_log - run_log - run_command - pump_until - - command_ok - command_fails - command_exit_is - program_help_ok - program_version_ok - program_options_handling_ok - command_like - command_like_safe - command_fails_like - command_checks_all - - $windows_os - $is_msys2 - $use_unix_sockets -); +use Exporter 'import'; 1; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index be90963..41bb582 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -149,6 +149,15 @@ of finding port numbers, registering instances for cleanup, etc. sub new { my ($class, $name, $pghost, $pgport) = @_; + + # Use release 15+ semantics when the arguments look like (node_name, + # %params). We can't use $class to decide, because get_new_node() passes + # a v14- argument list regardless of the class. $class might be an + # out-of-core subclass. $class->isa('PostgresNode') returns true even for + # descendants of PostgreSQL::Test::Cluster, so it doesn't help. + return $class->get_new_node(@_[ 1 .. $#_ ]) + if !$pghost or !$pgport or $pghost =~ /^[a-zA-Z0-9_]$/; + my $testname = basename($0); $testname =~ s/\.[^.]+$//; my $self = { @@ -2796,18 +2805,4 @@ sub corrupt_page_checksum =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Cluster; ## no critic (ProhibitMultiplePackages) - -sub new -{ - shift; # remove class param from args - return PostgresNode->get_new_node(@_); -} - -no warnings 'once'; - -*get_free_port = *PostgresNode::get_free_port; - 1; diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index f3ee20a..610050e 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -979,46 +979,4 @@ sub command_checks_all =cut -# support release 15+ perl module namespace - -package PostgreSQL::Test::Utils; ## no critic (ProhibitMultiplePackages) - -# we don't want to export anything here, but we want to support things called -# via this package name explicitly. - -# use typeglobs to alias these functions and variables - -no warnings qw(once); - -*generate_ascii_string = *TestLib::generate_ascii_string; -*slurp_dir = *TestLib::slurp_dir; -*slurp_file = *TestLib::slurp_file; -*append_to_file = *TestLib::append_to_file; -*check_mode_recursive = *TestLib::check_mode_recursive; -*chmod_recursive = *TestLib::chmod_recursive; -*check_pg_config = *TestLib::check_pg_config; -*dir_symlink = *TestLib::dir_symlink; -*system_or_bail = *TestLib::system_or_bail; -*system_log = *TestLib::system_log; -*run_log = *TestLib::run_log; -*run_command = *TestLib::run_command; -*command_ok = *TestLib::command_ok; -*command_fails = *TestLib::command_fails; -*command_exit_is = *TestLib::command_exit_is; -*program_help_ok = *TestLib::program_help_ok; -*program_version_ok = *TestLib::program_version_ok; -*program_options_handling_ok = *TestLib::program_options_handling_ok; -*command_like = *TestLib::command_like; -*command_like_safe = *TestLib::command_like_safe; -*command_fails_like = *TestLib::command_fails_like; -*command_checks_all = *TestLib::command_checks_all; - -*windows_os = *TestLib::windows_os; -*is_msys2 = *TestLib::is_msys2; -*use_unix_sockets = *TestLib::use_unix_sockets; -*timeout_default = *TestLib::timeout_default; -*tmp_check = *TestLib::tmp_check; -*log_path = *TestLib::log_path; -*test_logfile = *TestLib::test_log_file; - 1; ^ permalink raw reply [nested|flat] 23+ messages in thread
* Re: Postgres perl module namespace @ 2022-06-25 17:15 Noah Misch <[email protected]> parent: Noah Misch <[email protected]> 0 siblings, 0 replies; 23+ messages in thread From: Noah Misch @ 2022-06-25 17:15 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Erik Rijkers <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; Mark Dilger <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Jun 23, 2022 at 10:45:40PM -0700, Noah Misch wrote: > On Wed, Jun 22, 2022 at 11:03:22AM -0400, Andrew Dunstan wrote: > > On 2022-06-22 We 03:21, Noah Misch wrote: > > > On Tue, Apr 19, 2022 at 07:24:58PM -0400, Andrew Dunstan wrote: > > >> On 2022-04-19 Tu 18:39, Michael Paquier wrote: > > >>> +*generate_ascii_string = *TestLib::generate_ascii_string; > > >>> +*slurp_dir = *TestLib::slurp_dir; > > >>> +*slurp_file = *TestLib::slurp_file; > > >>> > > >>> I am not sure if it is possible and my perl-fu is limited in this > > >>> area, but could a failure be enforced when loading this path if a new > > >>> routine added in TestLib.pm is forgotten in this list? > > >> Not very easily that I'm aware of, but maybe some superior perl wizard > > >> will know better. > > > One can alias the symbol table, like https://metacpan.org/pod/Package::Alias > > > does. I'm attaching what I plan to use. Today, check-world fails after > > > > > > sed -i 's/TestLib/PostgreSQL::Test::Utils/g; s/PostgresNode/PostgreSQL::Test::Cluster/g' **/*.pl > > > > > > on REL_14_STABLE, because today's alias list is incomplete. With this change, > > > the same check-world passes. > > The patch wasn't sufficient to make that experiment pass for REL_10_STABLE, > where 017_shm.pl uses the %params argument of get_new_node(). The problem > call stack had PostgreSQL::Test::Cluster->get_new_code calling > PostgreSQL::Test::Cluster->new, which needs v14- semantics. Here's a fixed > version, just changing the new() hack. I pushed this, but it broke lapwing and wrasse. I will investigate. ^ permalink raw reply [nested|flat] 23+ messages in thread
end of thread, other threads:[~2022-06-25 17:15 UTC | newest] Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-11-29 01:43 Re: [HACKERS] Range Merge Join v1 Michael Paquier <[email protected]> 2021-07-30 22:35 [PATCH v3] Avoid creating archive status ".ready" files too early. Alvaro Herrera <[email protected]> 2021-07-30 22:35 [PATCH v2] Avoid creating archive status ".ready" files too early. Alvaro Herrera <[email protected]> 2021-08-17 03:52 [PATCH v12] Avoid creating archive status ".ready" files too early Nathan Bossart <[email protected]> 2021-08-17 03:52 [PATCH v10] Avoid creating archive status ".ready" files too early. Nathan Bossart <[email protected]> 2021-08-20 19:25 [PATCH v14] Avoid creating archive status ".ready" files too early Nathan Bossart <[email protected]> 2021-08-23 13:06 [PATCH v15] Avoid creating archive status ".ready" files too early Alvaro Herrera <[email protected]> 2022-04-18 18:07 Re: Postgres perl module namespace Tom Lane <[email protected]> 2022-04-18 19:29 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-19 15:36 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-19 17:15 ` Re: Postgres perl module namespace Andres Freund <[email protected]> 2022-04-19 20:06 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-19 22:39 ` Re: Postgres perl module namespace Michael Paquier <[email protected]> 2022-04-19 23:24 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-20 00:30 ` Re: Postgres perl module namespace Michael Paquier <[email protected]> 2022-04-20 19:56 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-21 04:11 ` Re: Postgres perl module namespace Michael Paquier <[email protected]> 2022-04-21 13:42 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-04-22 18:36 ` Re: Postgres perl module namespace Andres Freund <[email protected]> 2022-06-22 07:21 ` Re: Postgres perl module namespace Noah Misch <[email protected]> 2022-06-22 15:03 ` Re: Postgres perl module namespace Andrew Dunstan <[email protected]> 2022-06-24 05:45 ` Re: Postgres perl module namespace Noah Misch <[email protected]> 2022-06-25 17:15 ` Re: Postgres perl module namespace Noah Misch <[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