On Sun, Nov 8, 2015 at 9:50 PM, Michael Paquier
<[email protected]> wrote:
> On Sat, Nov 7, 2015 at 3:54 PM, Michael Paquier wrote:
>> I thought about something like that at some point by saving a minimum
>> activity pointer in XLogCtl, updated each time a segment was forcibly
>> switched or after inserting a checkpoint record. Then the bgwriter
>> looked at if the current insert position matched this minimum activity
>> pointer, skipping LogStandbySnapshot if both positions match. Does
>> this match your line of thoughts?
>
> Looking at the code, it occurred to me that the LSN position saved for
> a XLOG_SWITCH record is the last position of current segment, so we
> would still need to check if the current insert LSN matches the
> beginning of a new segment and if the last segment was forcibly
> switched by saving RecPtr of RequestXLogSwitch in XLogCtl for example.
> Thoughts?
I haven't given up on this patch yet, and putting again my head on
this problem I have finished with the patch attached, which checks if
the current insert LSN position is at the beginning of a segment that
has just been switched to decide if a standby snapshot should be
logged or not. This allows bringing back an idle system to the pre-9.3
state where a segment would be archived in the case of a low
archive_timeout only when a checkpoint has been issued on the system.
In order to achieve this idea I have added a field on XLogCtl that
saves the last time a segment has been forcibly switched after
XLOG_SWITCH.
Honestly I am failing to see why we should track the progress since
last checkpoint as mentioned upthread, and the current behavior is
certainly a regression.
Speaking of which, this patch was registered in this CF, I am moving
it to the next as a bug fix.
Regards,
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 147fd53..6608666 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -526,6 +526,8 @@ typedef struct XLogCtlData
XLogRecPtr RedoRecPtr; /* a recent copy of Insert->RedoRecPtr */
uint32 ckptXidEpoch; /* nextXID & epoch of latest checkpoint */
TransactionId ckptXid;
+ XLogRecPtr forcedSegSwitchLSN; /* LSN position of last forced segment
+ * switch */
XLogRecPtr asyncXactLSN; /* LSN of newest async commit/abort */
XLogRecPtr replicationSlotMinLSN; /* oldest LSN needed by any slot */
@@ -6315,6 +6317,7 @@ StartupXLOG(void)
checkPoint.newestCommitTs);
XLogCtl->ckptXidEpoch = checkPoint.nextXidEpoch;
XLogCtl->ckptXid = checkPoint.nextXid;
+ XLogCtl->forcedSegSwitchLSN = InvalidXLogRecPtr;
/*
* Initialize replication slots, before there's a chance to remove
@@ -8988,6 +8991,10 @@ RequestXLogSwitch(void)
XLogBeginInsert();
RecPtr = XLogInsert(RM_XLOG_ID, XLOG_SWITCH);
+ SpinLockAcquire(&XLogCtl->info_lck);
+ XLogCtl->forcedSegSwitchLSN = RecPtr;
+ SpinLockRelease(&XLogCtl->info_lck);
+
return RecPtr;
}
@@ -10628,6 +10635,21 @@ GetXLogWriteRecPtr(void)
}
/*
+ * Get last WAL position where an XLOG segment has been forcibly switched.
+ */
+XLogRecPtr
+GetXLogLastSwitchPtr(void)
+{
+ XLogRecPtr last_switch_lsn;
+
+ SpinLockAcquire(&XLogCtl->info_lck);
+ last_switch_lsn = XLogCtl->forcedSegSwitchLSN;
+ SpinLockRelease(&XLogCtl->info_lck);
+
+ return last_switch_lsn;
+}
+
+/*
* Returns the redo pointer of the last checkpoint or restartpoint. This is
* the oldest point in WAL that we still need, if we have to restart recovery.
*/
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 65465d6..ddd6efc 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -315,14 +315,28 @@ BackgroundWriterMain(void)
LOG_SNAPSHOT_INTERVAL_MS);
/*
- * only log if enough time has passed and some xlog record has
- * been inserted.
+ * Only log if enough time has passed and some xlog record has
+ * been inserted on a new segment. On an idle system where
+ * segments can be archived in a fast pace with for example a
+ * low archive_command setting, avoid as well logging a new
+ * standby snapshot if the current insert position is still
+ * at the beginning of the segment that has just been switched.
*/
- if (now >= timeout &&
- last_snapshot_lsn != GetXLogInsertRecPtr())
+ if (now >= timeout)
{
- last_snapshot_lsn = LogStandbySnapshot();
- last_snapshot_ts = now;
+ XLogRecPtr insert_lsn = GetXLogInsertRecPtr();
+ XLogRecPtr last_forced_switch_lsn = GetXLogLastSwitchPtr();
+ XLogSegNo insert_segno;
+
+ XLByteToSeg(insert_lsn, insert_segno);
+
+ if (last_snapshot_lsn != insert_lsn &&
+ !XLByteInPrevSeg(last_forced_switch_lsn, insert_segno) &&
+ (insert_lsn % XLOG_SEG_SIZE) != SizeOfXLogLongPHD)
+ {
+ last_snapshot_lsn = LogStandbySnapshot();
+ last_snapshot_ts = now;
+ }
}
}
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 790ca66..f6eb9c3 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -235,6 +235,7 @@ extern void GetXLogReceiptTime(TimestampTz *rtime, bool *fromStream);
extern XLogRecPtr GetXLogReplayRecPtr(TimeLineID *replayTLI);
extern XLogRecPtr GetXLogInsertRecPtr(void);
extern XLogRecPtr GetXLogWriteRecPtr(void);
+extern XLogRecPtr GetXLogLastSwitchPtr(void);
extern bool RecoveryIsPaused(void);
extern void SetRecoveryPause(bool recoveryPause);
extern TimestampTz GetLatestXTime(void);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
