PATCH: add pg_current_xlog_flush_location function
Hi,
attached is a patch adding a function pg_current_xlog_flush_location(),
which proved quite useful when investigating the ext4 data loss bug.
It's mostly what was already sent to that thread, except for docs that
were missing in the initial version.
I'll put this into 2016-01 CF.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
xlog-flush-func-v1.patchtext/x-diff; name=xlog-flush-func-v1.patchDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 60b9a09..0d67b82 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16755,6 +16755,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<primary>pg_create_restore_point</primary>
</indexterm>
<indexterm>
+ <primary>pg_current_xlog_flush_location</primary>
+ </indexterm>
+ <indexterm>
<primary>pg_current_xlog_insert_location</primary>
</indexterm>
<indexterm>
@@ -16811,6 +16814,13 @@ SELECT set_config('log_statement_stats', 'off', false);
</row>
<row>
<entry>
+ <literal><function>pg_current_xlog_flush_location()</function></literal>
+ </entry>
+ <entry><type>pg_lsn</type></entry>
+ <entry>Get current transaction log flush location</entry>
+ </row>
+ <row>
+ <entry>
<literal><function>pg_current_xlog_insert_location()</function></literal>
</entry>
<entry><type>pg_lsn</type></entry>
@@ -16943,13 +16953,14 @@ postgres=# select pg_start_backup('label_goes_here');
<function>pg_current_xlog_location</> displays the current transaction log write
location in the same format used by the above functions. Similarly,
<function>pg_current_xlog_insert_location</> displays the current transaction log
- insertion point. The insertion point is the <quote>logical</> end
- of the transaction log
- at any instant, while the write location is the end of what has actually
- been written out from the server's internal buffers. The write location
- is the end of what can be examined from outside the server, and is usually
+ insertion point and <function>pg_current_xlog_flush_location</> displays the
+ current transaction log flush point. The insertion point is the <quote>logical</>
+ end of the transaction log at any instant, while the write location is the end of
+ what has actually been written out from the server's internal buffers and flush
+ location is the location guaranteed to be written to durable storage. The write
+ location is the end of what can be examined from outside the server, and is usually
what you want if you are interested in archiving partially-complete transaction log
- files. The insertion point is made available primarily for server
+ files. The insertion and flush points are made available primarily for server
debugging purposes. These are both read-only operations and do not
require superuser permissions.
</para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 71fc8ff..40a17e8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10632,6 +10632,19 @@ GetXLogWriteRecPtr(void)
}
/*
+ * Get latest WAL flush pointer
+ */
+XLogRecPtr
+GetXLogFlushRecPtr(void)
+{
+ SpinLockAcquire(&XLogCtl->info_lck);
+ LogwrtResult = XLogCtl->LogwrtResult;
+ SpinLockRelease(&XLogCtl->info_lck);
+
+ return LogwrtResult.Flush;
+}
+
+/*
* 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/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 329bb8c..35c581d 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -195,7 +195,7 @@ pg_current_xlog_location(PG_FUNCTION_ARGS)
}
/*
- * Report the current WAL insert location (same format as pg_start_backup etc)
+ * Report the current WAL flush location (same format as pg_start_backup etc)
*
* This function is mostly for debugging purposes.
*/
@@ -216,6 +216,27 @@ pg_current_xlog_insert_location(PG_FUNCTION_ARGS)
}
/*
+ * Report the current WAL insert location (same format as pg_start_backup etc)
+ *
+ * This function is mostly for debugging purposes.
+ */
+Datum
+pg_current_xlog_flush_location(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr current_recptr;
+
+ if (RecoveryInProgress())
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is in progress"),
+ errhint("WAL control functions cannot be executed during recovery.")));
+
+ current_recptr = GetXLogFlushRecPtr();
+
+ PG_RETURN_LSN(current_recptr);
+}
+
+/*
* Report the last WAL receive location (same format as pg_start_backup etc)
*
* This is useful for determining how much of WAL is guaranteed to be received
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 790ca66..985291d 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 GetXLogFlushRecPtr(void);
extern bool RecoveryIsPaused(void);
extern void SetRecoveryPause(bool recoveryPause);
extern TimestampTz GetLatestXTime(void);
diff --git a/src/include/access/xlog_fn.h b/src/include/access/xlog_fn.h
index 3ebe966..f4575d7 100644
--- a/src/include/access/xlog_fn.h
+++ b/src/include/access/xlog_fn.h
@@ -19,6 +19,7 @@ extern Datum pg_switch_xlog(PG_FUNCTION_ARGS);
extern Datum pg_create_restore_point(PG_FUNCTION_ARGS);
extern Datum pg_current_xlog_location(PG_FUNCTION_ARGS);
extern Datum pg_current_xlog_insert_location(PG_FUNCTION_ARGS);
+extern Datum pg_current_xlog_flush_location(PG_FUNCTION_ARGS);
extern Datum pg_last_xlog_receive_location(PG_FUNCTION_ARGS);
extern Datum pg_last_xlog_replay_location(PG_FUNCTION_ARGS);
extern Datum pg_last_xact_replay_timestamp(PG_FUNCTION_ARGS);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index d8640db..ca8fcd4 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3154,6 +3154,8 @@ DATA(insert OID = 2849 ( pg_current_xlog_location PGNSP PGUID 12 1 0 0 0 f f f f
DESCR("current xlog write location");
DATA(insert OID = 2852 ( pg_current_xlog_insert_location PGNSP PGUID 12 1 0 0 0 f f f f t f v s 0 0 3220 "" _null_ _null_ _null_ _null_ _null_ pg_current_xlog_insert_location _null_ _null_ _null_ ));
DESCR("current xlog insert location");
+DATA(insert OID = 3330 ( pg_current_xlog_flush_location PGNSP PGUID 12 1 0 0 0 f f f f t f v s 0 0 3220 "" _null_ _null_ _null_ _null_ _null_ pg_current_xlog_flush_location _null_ _null_ _null_ ));
+DESCR("current xlog flush location");
DATA(insert OID = 2850 ( pg_xlogfile_name_offset PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 2249 "3220" "{3220,25,23}" "{i,o,o}" "{wal_location,file_name,file_offset}" _null_ _null_ pg_xlogfile_name_offset _null_ _null_ _null_ ));
DESCR("xlog filename and byte offset, given an xlog location");
DATA(insert OID = 2851 ( pg_xlogfile_name PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 25 "3220" _null_ _null_ _null_ _null_ _null_ pg_xlogfile_name _null_ _null_ _null_ ));
On Sun, Dec 13, 2015 at 12:07 AM, Tomas Vondra <tomas.vondra@2ndquadrant.com
wrote:
Hi,
attached is a patch adding a function pg_current_xlog_flush_location(),
which proved quite useful when investigating the ext4 data loss bug. It's
mostly what was already sent to that thread, except for docs that were
missing in the initial version.
/*
+ * Get latest WAL flush pointer
+ */
+XLogRecPtr
+GetXLogFlushRecPtr(void)
+{
+ SpinLockAcquire(&XLogCtl->info_lck);
+ LogwrtResult = XLogCtl->LogwrtResult;
+ SpinLockRelease(&XLogCtl->info_lck);
+
+ return LogwrtResult.Flush;
+}
+
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Hi,
On 12/13/2015 06:13 AM, Amit Kapila wrote:
...
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?
No, not really. I think I somehow missed that function when writing the
initial version of the patch. Will fix in v2 of the patch.
thanks
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
On 12/13/2015 08:38 PM, Tomas Vondra wrote:
Hi,
On 12/13/2015 06:13 AM, Amit Kapila wrote:
...
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?No, not really. I think I somehow missed that function when writing
the initial version of the patch. Will fix in v2 of the patch.
Hmm, so I've been looking at this, and I've realized that I've written
it like this because that's pretty much what pg_current_xlog_location()
does. It calls GetXLogWriteRecPtr which does this:
/*
* Get latest WAL write pointer
*/
XLogRecPtr
GetXLogWriteRecPtr(void)
{
SpinLockAcquire(&XLogCtl->info_lck);
LogwrtResult = XLogCtl->LogwrtResult;
SpinLockRelease(&XLogCtl->info_lck);
return LogwrtResult.Write;
}
so the patch does the same thing, except that I've returned "Flush".
OTOH GetFlushRecPtr does this:
XLogRecPtr
GetFlushRecPtr(void)
{
XLogRecPtr recptr;
SpinLockAcquire(&XLogCtl->info_lck);
recptr = XLogCtl->LogwrtResult.Flush;
SpinLockRelease(&XLogCtl->info_lck);
return recptr;
}
i.e. it does not update LogwrtResult, the local private copy. Not sure
what's appropriate here ...
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 11, 2016 at 3:29 AM, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:
Hi,
On 12/13/2015 08:38 PM, Tomas Vondra wrote:
Hi,
On 12/13/2015 06:13 AM, Amit Kapila wrote:
...
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?No, not really. I think I somehow missed that function when writing
the initial version of the patch. Will fix in v2 of the patch.Hmm, so I've been looking at this, and I've realized that I've written it
like this because that's pretty much what pg_current_xlog_location() does.
It calls GetXLogWriteRecPtr which does this:/*
* Get latest WAL write pointer
*/
XLogRecPtr
GetXLogWriteRecPtr(void)
{
SpinLockAcquire(&XLogCtl->info_lck);
LogwrtResult = XLogCtl->LogwrtResult;
SpinLockRelease(&XLogCtl->info_lck);return LogwrtResult.Write;
}so the patch does the same thing, except that I've returned "Flush".
OTOH GetFlushRecPtr does this:
XLogRecPtr
GetFlushRecPtr(void)
{
XLogRecPtr recptr;SpinLockAcquire(&XLogCtl->info_lck);
recptr = XLogCtl->LogwrtResult.Flush;
SpinLockRelease(&XLogCtl->info_lck);return recptr;
}i.e. it does not update LogwrtResult, the local private copy. Not sure
what's appropriate here ...
I think for the purpose of exposing the new API
pg_current_xlog_flush_location(), I see no reason why it has to
update the local variable LogwrtResult, although doing it either way
seems to be okay, however introducing new function
GetXLogFlushRecPtr() seems redundant. The internal function
(GetXLogInsertRecPtr()) used for API pg_current_xlog_insert_location()
doesn't update the local variable which indicates that calling the existing
function GetFlushRecPtr() is sufficient
for pg_current_xlog_flush_location().
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Mon, Jan 11, 2016 at 1:15 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Jan 11, 2016 at 3:29 AM, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:Hi,
On 12/13/2015 08:38 PM, Tomas Vondra wrote:
Hi,
On 12/13/2015 06:13 AM, Amit Kapila wrote:
...
Is there a reason why you can't use existing function
GetFlushRecPtr() in xlog.c?No, not really. I think I somehow missed that function when writing
the initial version of the patch. Will fix in v2 of the patch.Hmm, so I've been looking at this, and I've realized that I've written it
like this because that's pretty much what pg_current_xlog_location() does.
It calls GetXLogWriteRecPtr which does this:/*
* Get latest WAL write pointer
*/
XLogRecPtr
GetXLogWriteRecPtr(void)
{
SpinLockAcquire(&XLogCtl->info_lck);
LogwrtResult = XLogCtl->LogwrtResult;
SpinLockRelease(&XLogCtl->info_lck);return LogwrtResult.Write;
}so the patch does the same thing, except that I've returned "Flush".
OTOH GetFlushRecPtr does this:
XLogRecPtr
GetFlushRecPtr(void)
{
XLogRecPtr recptr;SpinLockAcquire(&XLogCtl->info_lck);
recptr = XLogCtl->LogwrtResult.Flush;
SpinLockRelease(&XLogCtl->info_lck);return recptr;
}i.e. it does not update LogwrtResult, the local private copy. Not sure
what's appropriate here ...I think for the purpose of exposing the new API
pg_current_xlog_flush_location(), I see no reason why it has to
update the local variable LogwrtResult, although doing it either way
seems to be okay, however introducing new function
GetXLogFlushRecPtr() seems redundant. The internal function
(GetXLogInsertRecPtr()) used for API pg_current_xlog_insert_location()
doesn't update the local variable which indicates that calling the existing
function GetFlushRecPtr() is sufficient for
pg_current_xlog_flush_location().
Updating LogwrtResult directly when calling your new function
GetXLogFlushRecPtr() does not strike me as a particularly good idea
per this portion in XLogFlush():
/* Quick exit if already known flushed */
if (record <= LogwrtResult.Flush)
return;
The same counts for GetXLogWriteRecPtr, we had better allocate the
value in an independent variable as there are checks using it. For now
it does not matter much for the write position because all the code
paths doing the checks explicitly update again the pointer before
looking at it but it seems to me that using an independent variable
would make the code more robust.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/11/2016 06:30 AM, Michael Paquier wrote:
Updating LogwrtResult directly when calling your new function
GetXLogFlushRecPtr() does not strike me as a particularly good idea
per this portion in XLogFlush():/* Quick exit if already known flushed */
if (record <= LogwrtResult.Flush)
return;The same counts for GetXLogWriteRecPtr, we had better allocate the
value in an independent variable as there are checks using it. For
now it does not matter much for the write position because all the
code paths doing the checks explicitly update again the pointer
before looking at it but it seems to me that using an independent
variable would make the code more robust.
Why? LogwrtResult only serves as a local cache of shared values, so
there should be no danger of skipping something.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11 January 2016 at 12:01, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:
On 01/11/2016 06:30 AM, Michael Paquier wrote:
Updating LogwrtResult directly when calling your new function
GetXLogFlushRecPtr() does not strike me as a particularly good idea
per this portion in XLogFlush():/* Quick exit if already known flushed */
if (record <= LogwrtResult.Flush)
return;The same counts for GetXLogWriteRecPtr, we had better allocate the
value in an independent variable as there are checks using it. For
now it does not matter much for the write position because all the
code paths doing the checks explicitly update again the pointer
before looking at it but it seems to me that using an independent
variable would make the code more robust.Why? LogwrtResult only serves as a local cache of shared values, so there
should be no danger of skipping something.
Comments in xlog.c say
"In addition to the shared variable, each backend has a private copy of
LogwrtResult, which is updated when convenient."
It is therefore valid to update the value of both Write and Flush positions
at the same time, any time either is required.
My suggested commit pattern for this is...
1. Update existing function to maintain LogwrtResult more eagerly (separate
patch)
2. Have the patch use the existing function name (main patch)
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jan 12, 2016 at 6:35 AM, Simon Riggs wrote:
Comments in xlog.c say
"In addition to the shared variable, each backend has a private copy of
LogwrtResult, which is updated when convenient."
It is therefore valid to update the value of both Write and Flush positions
at the same time, any time either is required.
Yes I saw this one yesterday when looking at this code. My comment
regarded the potential interactions between this field with XLogFlush,
but now I see that my concerns are not valid, updating more frequently
LogwrtResult may save some cycles though.
My suggested commit pattern for this is...
1. Update existing function to maintain LogwrtResult more eagerly (separate
patch)
The only place I see now that would benefit a bit from that is
UpdateMinRecoveryPoint when info_lck is taken, which can be called by
XLogFlush. Though I would expect this to have minimal impact.
2. Have the patch use the existing function name (main patch)
Yeah, we had better just use GetFlushRecPtr and be done with it. It
seems that there is little point to add a new function, and it is not
going to be called that much so its effects in updating LogwrtResult
would be minimized for a single backend.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 12 January 2016 at 05:58, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Tue, Jan 12, 2016 at 6:35 AM, Simon Riggs wrote:
Comments in xlog.c say
"In addition to the shared variable, each backend has a private copy of
LogwrtResult, which is updated when convenient."
It is therefore valid to update the value of both Write and Flushpositions
at the same time, any time either is required.
Yes I saw this one yesterday when looking at this code. My comment
regarded the potential interactions between this field with XLogFlush,
but now I see that my concerns are not valid, updating more frequently
LogwrtResult may save some cycles though.My suggested commit pattern for this is...
1. Update existing function to maintain LogwrtResult more eagerly(separate
patch)
The only place I see now that would benefit a bit from that is
UpdateMinRecoveryPoint when info_lck is taken, which can be called by
XLogFlush. Though I would expect this to have minimal impact.2. Have the patch use the existing function name (main patch)
Yeah, we had better just use GetFlushRecPtr and be done with it. It
seems that there is little point to add a new function, and it is not
going to be called that much so its effects in updating LogwrtResult
would be minimized for a single backend.
Patch committed, thanks for patch and review.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services