agora inbox for [email protected]
help / color / mirror / Atom feedFunction to promote standby servers
10+ messages / 5 participants
[nested] [flat]
* Function to promote standby servers
@ 2018-09-20 05:59 Laurenz Albe <[email protected]>
2018-09-21 05:27 ` Re: Function to promote standby servers Michael Paquier <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Laurenz Albe @ 2018-09-20 05:59 UTC (permalink / raw)
To: pgsql-hackers
Providing SQL access for administrative tasks seems to be a
good thing, see ALTER SYSTEM and pg_reload_conf().
In that vein, I propose a function pg_promote() to promote
physical standby servers.
If there are no fundamental objections, I'll add it to the
next commitfest.
Yours,
Laurenz Albe
Attachments:
[text/x-patch] 0001-Add-pg_promote-to-promote-standby-servers.patch (5.7K, ../../[email protected]/2-0001-Add-pg_promote-to-promote-standby-servers.patch)
download | inline diff:
From dd18d6eb38168db4d3d8c99a74d06b39e719092e Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email protected]>
Date: Thu, 20 Sep 2018 07:52:28 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 20 +++++++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlogfuncs.c | 48 ++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 4 +++
5 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4331bebc96..7beeaeacde 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18596,6 +18596,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18655,6 +18658,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote()</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function can only be
+ called by superusers and will only have an effect when run on
+ a standby server.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18692,6 +18705,13 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote()</function> sends a signal to the server that causes it
+ to leave standby mode. Since sending signals is by nature asynchronous,
+ successful execution of the function does not guarantee that the server has
+ already been promoted.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 8cb77f85ec..6014817d9e 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..9c55ef619b 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -35,6 +35,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +698,50 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ FILE *promote_file;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to promote standby servers")));
+
+ if (!RecoveryInProgress() || !StandbyMode)
+ PG_RETURN_BOOL(false);
+
+ /* create the promote signal file */
+ promote_file = AllocateFile("promote", "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create promote file: %m")));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write promote file: %m")));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink("promote");
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 860571440a..a26e5216da 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5997,6 +5997,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => '',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
--
2.17.1
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Function to promote standby servers
2018-09-20 05:59 Function to promote standby servers Laurenz Albe <[email protected]>
@ 2018-09-21 05:27 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Michael Paquier @ 2018-09-21 05:27 UTC (permalink / raw)
To: Laurenz Albe <[email protected]>; +Cc: pgsql-hackers
On Thu, Sep 20, 2018 at 07:59:02AM +0200, Laurenz Albe wrote:
> Providing SQL access for administrative tasks seems to be a
> good thing, see ALTER SYSTEM and pg_reload_conf().
>
> In that vein, I propose a function pg_promote() to promote
> physical standby servers.
No fundamental issues from me regarding the concept of being able to
trigger a promotion remotely, so +1. Do we want this capability as well
for fallback_promote? My gut tells me no, and that we ought to drop
this option at some point in the future, still that's worth mentioning.
You may want to move PROMOTE_SIGNAL_FILE in a position where xlogfuncs.c
could use it.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index ad0d505e54..23424886dc 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index be0c79d18c..9724ce20b8 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Fri_May_24_11_56_24_2019_374)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 6feca69126..f6d48368fe 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index be0c79d18c..9724ce20b8 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Wed_Jul_10_13_18_10_2019_842)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a3573ad0af..caaa09d785 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5dba86b8b8..7f119837ce 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Fri_Apr_26_17_40_34_2019_888)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v2-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a3573ad0af..caaa09d785 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5dba86b8b8..7f119837ce 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Thu_Apr_18_21_02_57_2019_406)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: MAX_BACKENDS size (comment accuracy)
@ 2025-02-24 11:42 Andres Freund <[email protected]>
2025-02-24 19:52 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Freund @ 2025-02-24 11:42 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: Jacob Brazeal <[email protected]>; pgsql-hackers
Hi,
On 2025-02-23 10:39:36 +1300, Thomas Munro wrote:
> On Sun, Feb 23, 2025 at 4:16 AM Andres Freund <[email protected]> wrote:
> > We do count the number of lwlock share lockers and the number of buffer
> > refcounts within those bits. And obviously 0 lockers/refcounts has to be
> > valid. So I think the limit is correct?
>
> Ah, right. That makes perfect sense. The 18 bits need to be able to
> hold a count, not just an index, and I confused myself about that from
> the moment I thought about the name PROC_NUMBER_BITS, which I retract.
Cool. I now pushed them, including static asserts in inval.c and deadlock.
Thanks for the reviews and the complaint leading to these changes.
> > I didn't yet have enough coffe, but isn't the inval.c limit 2^24-1 rather than
> > 2^23-1?
>
> Yeah, it has 24 bits of space, but curiously backend_hi is signed, so
> (msg->sm.backend_hi << 16) would be sign-extended, so it wouldn't actually
> work if you used all 24 bits... which is obviously not a real
> problem...
Heh, that's odd. I left it like that, didn't seem worth changing given that
it's so far from the real limit...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: MAX_BACKENDS size (comment accuracy)
2025-02-24 11:42 Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
@ 2025-02-24 19:52 ` Nathan Bossart <[email protected]>
2025-02-24 20:38 ` Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Nathan Bossart @ 2025-02-24 19:52 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jacob Brazeal <[email protected]>; pgsql-hackers
The recent commits for this drew my attention to the comment for
MAX_BACKENDS. Specifically, it says we check the value in
RegisterBackgroundWorker() (which appears to have been untrue since we
added max_worker_processes) and relevant GUC check hooks (which I removed
last year in commit 0b1fe14). I wrote a short patch to fix this, which I
intend to commit soon unless there is feedback.
--
nathan
From 5038a5256942999bd544879d25312338686dddf2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 24 Feb 2025 13:40:30 -0600
Subject: [PATCH v1 1/1] Fix comment for MAX_BACKENDS.
This comment mentions that we check the configured number of
backends does not exceed MAX_BACKENDS in RegisterBackgroundWorker()
and relevant GUC check hooks, neither of which is true anymore. To
fix, adjust this comment to state that we do the check in
InitializeMaxBackends().
Oversights in commits 6bc8ef0b7f and 0b1fe1413e.
---
src/include/storage/procnumber.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/storage/procnumber.h b/src/include/storage/procnumber.h
index 75c2c7a17c0..2ddaaf0c646 100644
--- a/src/include/storage/procnumber.h
+++ b/src/include/storage/procnumber.h
@@ -32,8 +32,8 @@ typedef int ProcNumber;
* currently realistic configurations. Even if that limitation were removed,
* we still could not a) exceed 2^23-1 because inval.c stores the ProcNumber
* as a 3-byte signed integer, b) INT_MAX/4 because some places compute
- * 4*MaxBackends without any overflow check. This is rechecked in the
- * relevant GUC check hooks and in RegisterBackgroundWorker().
+ * 4*MaxBackends without any overflow check. We check that the configured
+ * number of backends does not exceed MAX_BACKENDS in InitializeMaxBackends().
*/
#define MAX_BACKENDS_BITS 18
#define MAX_BACKENDS ((1U << MAX_BACKENDS_BITS)-1)
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v1-0001-Fix-comment-for-MAX_BACKENDS.patch (1.5K, ../../Z7zOEzz8lNjaU9yf@nathan/2-v1-0001-Fix-comment-for-MAX_BACKENDS.patch)
download | inline diff:
From 5038a5256942999bd544879d25312338686dddf2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 24 Feb 2025 13:40:30 -0600
Subject: [PATCH v1 1/1] Fix comment for MAX_BACKENDS.
This comment mentions that we check the configured number of
backends does not exceed MAX_BACKENDS in RegisterBackgroundWorker()
and relevant GUC check hooks, neither of which is true anymore. To
fix, adjust this comment to state that we do the check in
InitializeMaxBackends().
Oversights in commits 6bc8ef0b7f and 0b1fe1413e.
---
src/include/storage/procnumber.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/storage/procnumber.h b/src/include/storage/procnumber.h
index 75c2c7a17c0..2ddaaf0c646 100644
--- a/src/include/storage/procnumber.h
+++ b/src/include/storage/procnumber.h
@@ -32,8 +32,8 @@ typedef int ProcNumber;
* currently realistic configurations. Even if that limitation were removed,
* we still could not a) exceed 2^23-1 because inval.c stores the ProcNumber
* as a 3-byte signed integer, b) INT_MAX/4 because some places compute
- * 4*MaxBackends without any overflow check. This is rechecked in the
- * relevant GUC check hooks and in RegisterBackgroundWorker().
+ * 4*MaxBackends without any overflow check. We check that the configured
+ * number of backends does not exceed MAX_BACKENDS in InitializeMaxBackends().
*/
#define MAX_BACKENDS_BITS 18
#define MAX_BACKENDS ((1U << MAX_BACKENDS_BITS)-1)
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: MAX_BACKENDS size (comment accuracy)
2025-02-24 11:42 Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
2025-02-24 19:52 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[email protected]>
@ 2025-02-24 20:38 ` Andres Freund <[email protected]>
2025-02-24 21:02 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Freund @ 2025-02-24 20:38 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jacob Brazeal <[email protected]>; pgsql-hackers
Hi,
On 2025-02-24 13:52:51 -0600, Nathan Bossart wrote:
> The recent commits for this drew my attention to the comment for
> MAX_BACKENDS. Specifically, it says we check the value in
> RegisterBackgroundWorker() (which appears to have been untrue since we
> added max_worker_processes) and relevant GUC check hooks (which I removed
> last year in commit 0b1fe14). I wrote a short patch to fix this, which I
> intend to commit soon unless there is feedback.
Makes sense.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: MAX_BACKENDS size (comment accuracy)
2025-02-24 11:42 Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
2025-02-24 19:52 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[email protected]>
2025-02-24 20:38 ` Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
@ 2025-02-24 21:02 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Nathan Bossart @ 2025-02-24 21:02 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Jacob Brazeal <[email protected]>; pgsql-hackers
On Mon, Feb 24, 2025 at 03:38:24PM -0500, Andres Freund wrote:
> Makes sense.
Committed, thanks.
--
nathan
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2025-02-24 21:02 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-09-20 05:59 Function to promote standby servers Laurenz Albe <[email protected]>
2018-09-21 05:27 ` Michael Paquier <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2025-02-24 11:42 Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
2025-02-24 19:52 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[email protected]>
2025-02-24 20:38 ` Re: MAX_BACKENDS size (comment accuracy) Andres Freund <[email protected]>
2025-02-24 21:02 ` Re: MAX_BACKENDS size (comment accuracy) Nathan Bossart <[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