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.

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 7781c56f0eb28b5924f314977d245b6b49a7f938..12471d1c6e1c02662a8bd92de7fd3a5acff5ca8a 100644 (file)
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5450,10 +5450,11 @@
       <entry><structfield>local_lsn</structfield></entry>
       <entry><type>pg_lsn</type></entry>
       <entry></entry>
-      <entry>This node's LSN that at
-      which <literal>remote_lsn</literal> has been replicated. Used to
-      flush commit records before persisting data to disk when using
-      asynchronous commits.</entry>
+      <entry>
+       This node's LSN at which <literal>remote_lsn</literal> has
+       been replicated. Used to flush commit records before persisting
+       data to disk when using asynchronous commits.
+      </entry>
      </row>
     </tbody>
    </tgroup>

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9635cf291e1b782bd4343b14d814fb45e929e188..4fcc4fe5aaaf77b06b8ef9b4cac29bc1cab75d93 100644 (file)
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17441,7 +17441,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
         <indexterm>
          <primary>pg_replication_origin_session_progress</primary>
         </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>
         pg_lsn

diff --git a/doc/src/sgml/replication-origins.sgml b/doc/src/sgml/replication-origins.sgml
index 40fcc6d3d0aaa7f321c857f282418c1765b34011..67f512207e333cb1191b7c67e88ce7abdec26f18 100644 (file)
--- a/doc/src/sgml/replication-origins.sgml
+++ b/doc/src/sgml/replication-origins.sgml
@@ -57,7 +57,7 @@
   Using the replication origin infrastructure a session can be
   marked as replaying from a remote node (using the
   <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
   transaction basis using
   <link linkend="pg-replication-origin-xact-setup"><function>pg_replication_origin_xact_setup()</function></link>.

diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 0b89c0a7a2c3fe9a0e7ba41d914947f375e2a8d9..abd8420da69fd0bde72318fa99b9d9d3cce7bcfa 100644 (file)
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -73,7 +73,7 @@ static XLogRecData *mainrdata_head;
 static XLogRecData *mainrdata_last = (XLogRecData *) &mainrdata_head;
 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;
 
 /*

diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index f4ba86e83692669d4ff78831d997aed3cd4c4b5a..c219590a9bbe29cfcc2c9526d0c347d452c9368c 100644 (file)
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -313,7 +313,7 @@ replorigin_create(char *roname)
    if (tuple == NULL)
        ereport(ERROR,
                (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);
    return roident;
@@ -375,6 +375,10 @@ replorigin_drop(RepOriginId roident)
    LWLockRelease(ReplicationOriginLock);
 
    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);
    ReleaseSysCache(tuple);
 
@@ -437,7 +441,7 @@ ReplicationOriginShmemSize(void)
    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
     * sufficient to reuse it, lest we introduce a separate guc.
     */
@@ -523,7 +527,7 @@ CheckPointReplicationOrigin(void)
        ereport(PANIC,
                (errcode_for_file_access(),
                 errmsg("could not remove file \"%s\": %m",
-                       path)));
+                       tmppath)));
 
    /*
     * no other backend can perform this at the same time, we're protected by
@@ -799,12 +803,12 @@ replorigin_redo(XLogReaderState *record)
  * Tell the replication origin progress machinery that a commit from 'node'
  * 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
- * setting up replorigin_sesssion_origin_lsn and replorigin_sesssion_origin that ensures we
- * won't loose knowledge about that after a crash if the transaction had a
- * persistent effect (think of asynchronous commits).
+ * setting up replorigin_sesssion_origin_lsn and replorigin_sesssion_origin
+ * that ensures we won't loose knowledge about that after a crash if the
+ * 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
- * 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,
  * unless running in recovery.
@@ -1249,7 +1253,6 @@ pg_replication_origin_session_reset(PG_FUNCTION_ARGS)
 
    replorigin_session_reset();
 
-   /* FIXME */
    replorigin_sesssion_origin = InvalidRepOriginId;
    replorigin_sesssion_origin_lsn = InvalidXLogRecPtr;
    replorigin_sesssion_origin_timestamp = 0;