Commit 18e86135 authored by Andres Freund's avatar Andres Freund

Address points made in post-commit review of replication origins.

Amit reviewed the replication origins patch and made some good
points. Address them. This fixes typos in error messages, docs and
comments and adds a missing error check (although in a
should-never-happen scenario).

Discussion: CAA4eK1JqUBVeWWKwUmBPryFaje4190ug0y-OAUHWQ6tD83V4xg@mail.gmail.com
Backpatch: 9.5, where replication origins were introduced.
parent d6a8c943
...@@ -5450,10 +5450,11 @@ ...@@ -5450,10 +5450,11 @@
<entry><structfield>local_lsn</structfield></entry> <entry><structfield>local_lsn</structfield></entry>
<entry><type>pg_lsn</type></entry> <entry><type>pg_lsn</type></entry>
<entry></entry> <entry></entry>
<entry>This node's LSN that at <entry>
which <literal>remote_lsn</literal> has been replicated. Used to This node's LSN at which <literal>remote_lsn</literal> has
flush commit records before persisting data to disk when using been replicated. Used to flush commit records before persisting
asynchronous commits.</entry> data to disk when using asynchronous commits.
</entry>
</row> </row>
</tbody> </tbody>
</tgroup> </tgroup>
......
...@@ -17441,7 +17441,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); ...@@ -17441,7 +17441,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
<indexterm> <indexterm>
<primary>pg_replication_origin_session_progress</primary> <primary>pg_replication_origin_session_progress</primary>
</indexterm> </indexterm>
<literal><function>pg_replication_origin_progress(<parameter>flush</parameter> <type>bool</type>)</function></literal> <literal><function>pg_replication_origin_session_progress(<parameter>flush</parameter> <type>bool</type>)</function></literal>
</entry> </entry>
<entry> <entry>
pg_lsn pg_lsn
......
...@@ -57,7 +57,7 @@ ...@@ -57,7 +57,7 @@
Using the replication origin infrastructure a session can be Using the replication origin infrastructure a session can be
marked as replaying from a remote node (using the marked as replaying from a remote node (using the
<link linkend="pg-replication-origin-session-setup"><function>pg_replication_origin_session_setup()</function></link> <link linkend="pg-replication-origin-session-setup"><function>pg_replication_origin_session_setup()</function></link>
function. Additionally the <acronym>LSN</acronym> and commit function). Additionally the <acronym>LSN</acronym> and commit
timestamp of every source transaction can be configured on a per timestamp of every source transaction can be configured on a per
transaction basis using transaction basis using
<link linkend="pg-replication-origin-xact-setup"><function>pg_replication_origin_xact_setup()</function></link>. <link linkend="pg-replication-origin-xact-setup"><function>pg_replication_origin_xact_setup()</function></link>.
......
...@@ -73,7 +73,7 @@ static XLogRecData *mainrdata_head; ...@@ -73,7 +73,7 @@ static XLogRecData *mainrdata_head;
static XLogRecData *mainrdata_last = (XLogRecData *) &mainrdata_head; static XLogRecData *mainrdata_last = (XLogRecData *) &mainrdata_head;
static uint32 mainrdata_len; /* total # of bytes in chain */ static uint32 mainrdata_len; /* total # of bytes in chain */
/* Should te in-progress insertion log the origin */ /* Should the in-progress insertion log the origin? */
static bool include_origin = false; static bool include_origin = false;
/* /*
......
...@@ -313,7 +313,7 @@ replorigin_create(char *roname) ...@@ -313,7 +313,7 @@ replorigin_create(char *roname)
if (tuple == NULL) if (tuple == NULL)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("no free replication oid could be found"))); errmsg("no free replication origin oid could be found")));
heap_freetuple(tuple); heap_freetuple(tuple);
return roident; return roident;
...@@ -375,6 +375,10 @@ replorigin_drop(RepOriginId roident) ...@@ -375,6 +375,10 @@ replorigin_drop(RepOriginId roident)
LWLockRelease(ReplicationOriginLock); LWLockRelease(ReplicationOriginLock);
tuple = SearchSysCache1(REPLORIGIDENT, ObjectIdGetDatum(roident)); tuple = SearchSysCache1(REPLORIGIDENT, ObjectIdGetDatum(roident));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for replication origin with oid %u",
roident);
simple_heap_delete(rel, &tuple->t_self); simple_heap_delete(rel, &tuple->t_self);
ReleaseSysCache(tuple); ReleaseSysCache(tuple);
...@@ -437,7 +441,7 @@ ReplicationOriginShmemSize(void) ...@@ -437,7 +441,7 @@ ReplicationOriginShmemSize(void)
Size size = 0; Size size = 0;
/* /*
* XXX: max_replication_slots is arguablethe wrong thing to use here, here * XXX: max_replication_slots is arguably the wrong thing to use, as here
* we keep the replay state of *remote* transactions. But for now it seems * we keep the replay state of *remote* transactions. But for now it seems
* sufficient to reuse it, lest we introduce a separate guc. * sufficient to reuse it, lest we introduce a separate guc.
*/ */
...@@ -523,7 +527,7 @@ CheckPointReplicationOrigin(void) ...@@ -523,7 +527,7 @@ CheckPointReplicationOrigin(void)
ereport(PANIC, ereport(PANIC,
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not remove file \"%s\": %m", errmsg("could not remove file \"%s\": %m",
path))); tmppath)));
/* /*
* no other backend can perform this at the same time, we're protected by * no other backend can perform this at the same time, we're protected by
...@@ -799,12 +803,12 @@ replorigin_redo(XLogReaderState *record) ...@@ -799,12 +803,12 @@ replorigin_redo(XLogReaderState *record)
* Tell the replication origin progress machinery that a commit from 'node' * Tell the replication origin progress machinery that a commit from 'node'
* that originated at the LSN remote_commit on the remote node was replayed * that originated at the LSN remote_commit on the remote node was replayed
* successfully and that we don't need to do so again. In combination with * successfully and that we don't need to do so again. In combination with
* setting up replorigin_sesssion_origin_lsn and replorigin_sesssion_origin that ensures we * setting up replorigin_sesssion_origin_lsn and replorigin_sesssion_origin
* won't loose knowledge about that after a crash if the transaction had a * that ensures we won't loose knowledge about that after a crash if the
* persistent effect (think of asynchronous commits). * transaction had a persistent effect (think of asynchronous commits).
* *
* local_commit needs to be a local LSN of the commit so that we can make sure * local_commit needs to be a local LSN of the commit so that we can make sure
* uppon a checkpoint that enough WAL has been persisted to disk. * upon a checkpoint that enough WAL has been persisted to disk.
* *
* Needs to be called with a RowExclusiveLock on pg_replication_origin, * Needs to be called with a RowExclusiveLock on pg_replication_origin,
* unless running in recovery. * unless running in recovery.
...@@ -1249,7 +1253,6 @@ pg_replication_origin_session_reset(PG_FUNCTION_ARGS) ...@@ -1249,7 +1253,6 @@ pg_replication_origin_session_reset(PG_FUNCTION_ARGS)
replorigin_session_reset(); replorigin_session_reset();
/* FIXME */
replorigin_sesssion_origin = InvalidRepOriginId; replorigin_sesssion_origin = InvalidRepOriginId;
replorigin_sesssion_origin_lsn = InvalidXLogRecPtr; replorigin_sesssion_origin_lsn = InvalidXLogRecPtr;
replorigin_sesssion_origin_timestamp = 0; replorigin_sesssion_origin_timestamp = 0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment