From 37163e22bddb30a235c9748f49ad54d5e99db142 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 7 Aug 2015 15:08:51 +0200 Subject: [PATCH] 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. --- doc/src/sgml/catalogs.sgml | 9 +++++---- doc/src/sgml/func.sgml | 2 +- doc/src/sgml/replication-origins.sgml | 2 +- src/backend/access/transam/xloginsert.c | 2 +- src/backend/replication/logical/origin.c | 19 +++++++++++-------- 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 7781c56f0e..12471d1c6e 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -5450,10 +5450,11 @@ local_lsn pg_lsn - This node's LSN that at - which remote_lsn has been replicated. Used to - flush commit records before persisting data to disk when using - asynchronous commits. + + This node's LSN at which remote_lsn has + been replicated. Used to flush commit records before persisting + data to disk when using asynchronous commits. + diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 2e1ac2f070..1bd72485ac 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -17421,7 +17421,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); pg_replication_origin_session_progress - pg_replication_origin_progress(flush bool) + pg_replication_origin_session_progress(flush bool) pg_lsn diff --git a/doc/src/sgml/replication-origins.sgml b/doc/src/sgml/replication-origins.sgml index 40fcc6d3d0..67f512207e 100644 --- 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 pg_replication_origin_session_setup() - function. Additionally the LSN and commit + function). Additionally the LSN and commit timestamp of every source transaction can be configured on a per transaction basis using pg_replication_origin_xact_setup(). diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 0b89c0a7a2..abd8420da6 100644 --- 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 f4ba86e836..c219590a9b 100644 --- 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;