convert elog(LOG) calls to ereport

Started by Peter Eisentrautabout 5 years ago9 messages
#1Peter Eisentraut
peter.eisentraut@enterprisedb.com
1 attachment(s)

There are a number of elog(LOG) calls that appear to be user-facing, so
they should be ereport()s. This patch changes them. There are more
elog(LOG) calls remaining, but they all appear to be some kind of
debugging support. Also, I changed a few elog(FATAL)s that were nearby,
but I didn't specifically look for them.

Attachments:

0001-Convert-elog-LOG-calls-to-ereport-where-appropriate.patchtext/plain; charset=UTF-8; name=0001-Convert-elog-LOG-calls-to-ereport-where-appropriate.patch; x-mac-creator=0; x-mac-type=0Download
From 05e5cf58e66072202707c6fddcd59768a26039be Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 2 Dec 2020 13:16:15 +0100
Subject: [PATCH] Convert elog(LOG) calls to ereport() where appropriate

User-visible log messages should go through ereport(), so they are
subject to translation.  Many remaining elog(LOG) calls are really
debugging calls.
---
 src/backend/access/gist/gist.c           |  5 +-
 src/backend/access/nbtree/nbtpage.c      |  7 +-
 src/backend/access/transam/xlog.c        | 98 ++++++++++++++++--------
 src/backend/commands/variable.c          |  3 +-
 src/backend/libpq/auth.c                 |  7 +-
 src/backend/libpq/hba.c                  |  3 +-
 src/backend/libpq/pqcomm.c               | 56 +++++++++-----
 src/backend/optimizer/geqo/geqo_erx.c    |  9 ++-
 src/backend/port/posix_sema.c            | 18 +++--
 src/backend/port/sysv_sema.c             | 12 ++-
 src/backend/port/sysv_shmem.c            | 55 +++++++------
 src/backend/port/win32_shmem.c           | 71 ++++++++++-------
 src/backend/postmaster/bgworker.c        |  8 +-
 src/backend/postmaster/pgstat.c          | 20 +++--
 src/backend/postmaster/postmaster.c      | 44 +++++++----
 src/backend/replication/logical/origin.c |  9 ++-
 src/backend/storage/file/fd.c            |  6 +-
 src/backend/utils/misc/guc.c             |  2 +-
 18 files changed, 274 insertions(+), 159 deletions(-)

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 25b42e38f2..3f2b416ce1 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -1167,8 +1167,9 @@ gistfixsplit(GISTInsertState *state, GISTSTATE *giststate)
 	Page		page;
 	List	   *splitinfo = NIL;
 
-	elog(LOG, "fixing incomplete split in index \"%s\", block %u",
-		 RelationGetRelationName(state->r), stack->blkno);
+	ereport(LOG,
+			(errmsg("fixing incomplete split in index \"%s\", block %u",
+					RelationGetRelationName(state->r), stack->blkno)));
 
 	Assert(GistFollowRight(stack->page));
 	Assert(OffsetNumberIsValid(stack->downlinkoffnum));
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 848123d921..793434c026 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2151,9 +2151,10 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
 
 			if (leftsib == P_NONE)
 			{
-				elog(LOG, "no left sibling (concurrent deletion?) of block %u in \"%s\"",
-					 target,
-					 RelationGetRelationName(rel));
+				ereport(LOG,
+						(errmsg("no left sibling (concurrent deletion?) of block %u in \"%s\"",
+								target,
+								RelationGetRelationName(rel))));
 				if (target != leafblkno)
 				{
 					/* we have only a pin on target, but pin+lock on leafbuf */
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 13f1d8c3dc..bf9c77cdcb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1811,9 +1811,10 @@ WaitXLogInsertionsToFinish(XLogRecPtr upto)
 	 */
 	if (upto > reservedUpto)
 	{
-		elog(LOG, "request to flush past end of generated WAL; request %X/%X, currpos %X/%X",
-			 (uint32) (upto >> 32), (uint32) upto,
-			 (uint32) (reservedUpto >> 32), (uint32) reservedUpto);
+		ereport(LOG,
+				(errmsg("request to flush past end of generated WAL; request %X/%X, current position %X/%X",
+						(uint32) (upto >> 32), (uint32) upto,
+						(uint32) (reservedUpto >> 32), (uint32) reservedUpto)));
 		upto = reservedUpto;
 	}
 
@@ -8532,16 +8533,28 @@ ShutdownXLOG(int code, Datum arg)
 static void
 LogCheckpointStart(int flags, bool restartpoint)
 {
-	elog(LOG, "%s starting:%s%s%s%s%s%s%s%s",
-		 restartpoint ? "restartpoint" : "checkpoint",
-		 (flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
-		 (flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
-		 (flags & CHECKPOINT_IMMEDIATE) ? " immediate" : "",
-		 (flags & CHECKPOINT_FORCE) ? " force" : "",
-		 (flags & CHECKPOINT_WAIT) ? " wait" : "",
-		 (flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
-		 (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
-		 (flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" : "");
+	if (restartpoint)
+		ereport(LOG,
+				(errmsg("restartpoint starting:%s%s%s%s%s%s%s%s",
+						(flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
+						(flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
+						(flags & CHECKPOINT_IMMEDIATE) ? " immediate" : "",
+						(flags & CHECKPOINT_FORCE) ? " force" : "",
+						(flags & CHECKPOINT_WAIT) ? " wait" : "",
+						(flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
+						(flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
+						(flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" : "")));
+	else
+		ereport(LOG,
+				(errmsg("checkpoint starting:%s%s%s%s%s%s%s%s",
+						(flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
+						(flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
+						(flags & CHECKPOINT_IMMEDIATE) ? " immediate" : "",
+						(flags & CHECKPOINT_FORCE) ? " force" : "",
+						(flags & CHECKPOINT_WAIT) ? " wait" : "",
+						(flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
+						(flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
+						(flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" : "")));
 }
 
 /*
@@ -8591,25 +8604,46 @@ LogCheckpointEnd(bool restartpoint)
 			CheckpointStats.ckpt_sync_rels;
 	average_msecs = (long) ((average_sync_time + 999) / 1000);
 
-	elog(LOG, "%s complete: wrote %d buffers (%.1f%%); "
-		 "%d WAL file(s) added, %d removed, %d recycled; "
-		 "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
-		 "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
-		 "distance=%d kB, estimate=%d kB",
-		 restartpoint ? "restartpoint" : "checkpoint",
-		 CheckpointStats.ckpt_bufs_written,
-		 (double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
-		 CheckpointStats.ckpt_segs_added,
-		 CheckpointStats.ckpt_segs_removed,
-		 CheckpointStats.ckpt_segs_recycled,
-		 write_msecs / 1000, (int) (write_msecs % 1000),
-		 sync_msecs / 1000, (int) (sync_msecs % 1000),
-		 total_msecs / 1000, (int) (total_msecs % 1000),
-		 CheckpointStats.ckpt_sync_rels,
-		 longest_msecs / 1000, (int) (longest_msecs % 1000),
-		 average_msecs / 1000, (int) (average_msecs % 1000),
-		 (int) (PrevCheckPointDistance / 1024.0),
-		 (int) (CheckPointDistanceEstimate / 1024.0));
+	if (restartpoint)
+		ereport(LOG,
+				(errmsg("restartpoint complete: wrote %d buffers (%.1f%%); "
+						"%d WAL file(s) added, %d removed, %d recycled; "
+						"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
+						"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
+						"distance=%d kB, estimate=%d kB",
+						CheckpointStats.ckpt_bufs_written,
+						(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+						CheckpointStats.ckpt_segs_added,
+						CheckpointStats.ckpt_segs_removed,
+						CheckpointStats.ckpt_segs_recycled,
+						write_msecs / 1000, (int) (write_msecs % 1000),
+						sync_msecs / 1000, (int) (sync_msecs % 1000),
+						total_msecs / 1000, (int) (total_msecs % 1000),
+						CheckpointStats.ckpt_sync_rels,
+						longest_msecs / 1000, (int) (longest_msecs % 1000),
+						average_msecs / 1000, (int) (average_msecs % 1000),
+						(int) (PrevCheckPointDistance / 1024.0),
+						(int) (CheckPointDistanceEstimate / 1024.0))));
+	else
+		ereport(LOG,
+				(errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
+						"%d WAL file(s) added, %d removed, %d recycled; "
+						"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
+						"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
+						"distance=%d kB, estimate=%d kB",
+						CheckpointStats.ckpt_bufs_written,
+						(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+						CheckpointStats.ckpt_segs_added,
+						CheckpointStats.ckpt_segs_removed,
+						CheckpointStats.ckpt_segs_recycled,
+						write_msecs / 1000, (int) (write_msecs % 1000),
+						sync_msecs / 1000, (int) (sync_msecs % 1000),
+						total_msecs / 1000, (int) (total_msecs % 1000),
+						CheckpointStats.ckpt_sync_rels,
+						longest_msecs / 1000, (int) (longest_msecs % 1000),
+						average_msecs / 1000, (int) (average_msecs % 1000),
+						(int) (PrevCheckPointDistance / 1024.0),
+						(int) (CheckPointDistanceEstimate / 1024.0))));
 }
 
 /*
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 484f7ea2c0..66b7ebb02e 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -725,7 +725,8 @@ assign_client_encoding(const char *newval, void *extra)
 
 	/* We do not expect an error if PrepareClientEncoding succeeded */
 	if (SetClientEncoding(encoding) < 0)
-		elog(LOG, "SetClientEncoding(%d) failed", encoding);
+		ereport(LOG,
+				(errmsg("SetClientEncoding(%d) failed", encoding)));
 }
 
 
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index d132c5cb48..3d80930968 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2131,9 +2131,10 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message **msg,
 				reply[i].resp_retcode = PAM_SUCCESS;
 				break;
 			default:
-				elog(LOG, "unsupported PAM conversation %d/\"%s\"",
-					 msg[i]->msg_style,
-					 msg[i]->msg ? msg[i]->msg : "(none)");
+				ereport(LOG,
+						(errmsg("unsupported PAM conversation %d/\"%s\"",
+								msg[i]->msg_style,
+								msg[i]->msg ? msg[i]->msg : "(none)")));
 				goto fail;
 		}
 	}
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 3a78d2043e..0cc4397769 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -857,7 +857,8 @@ check_same_host_or_net(SockAddr *raddr, IPCompareMethod method)
 	errno = 0;
 	if (pg_foreach_ifaddr(check_network_callback, &cn) < 0)
 	{
-		elog(LOG, "error enumerating network interfaces: %m");
+		ereport(LOG,
+				(errmsg("error enumerating network interfaces: %m")));
 		return false;
 	}
 
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 64a351cedc..04f6489d95 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -750,7 +750,8 @@ StreamConnection(pgsocket server_fd, Port *port)
 					(struct sockaddr *) &port->laddr.addr,
 					&port->laddr.salen) < 0)
 	{
-		elog(LOG, "getsockname() failed: %m");
+		ereport(LOG,
+				(errmsg("getsockname() failed: %m")));
 		return STATUS_ERROR;
 	}
 
@@ -769,7 +770,8 @@ StreamConnection(pgsocket server_fd, Port *port)
 		if (setsockopt(port->sock, IPPROTO_TCP, TCP_NODELAY,
 					   (char *) &on, sizeof(on)) < 0)
 		{
-			elog(LOG, "setsockopt(%s) failed: %m", "TCP_NODELAY");
+			ereport(LOG,
+					(errmsg("setsockopt(%s) failed: %m", "TCP_NODELAY")));
 			return STATUS_ERROR;
 		}
 #endif
@@ -777,7 +779,8 @@ StreamConnection(pgsocket server_fd, Port *port)
 		if (setsockopt(port->sock, SOL_SOCKET, SO_KEEPALIVE,
 					   (char *) &on, sizeof(on)) < 0)
 		{
-			elog(LOG, "setsockopt(%s) failed: %m", "SO_KEEPALIVE");
+			ereport(LOG,
+					(errmsg("setsockopt(%s) failed: %m", "SO_KEEPALIVE")));
 			return STATUS_ERROR;
 		}
 
@@ -808,7 +811,8 @@ StreamConnection(pgsocket server_fd, Port *port)
 		if (getsockopt(port->sock, SOL_SOCKET, SO_SNDBUF, (char *) &oldopt,
 					   &optlen) < 0)
 		{
-			elog(LOG, "getsockopt(%s) failed: %m", "SO_SNDBUF");
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", "SO_SNDBUF")));
 			return STATUS_ERROR;
 		}
 		newopt = PQ_SEND_BUFFER_SIZE * 4;
@@ -817,7 +821,8 @@ StreamConnection(pgsocket server_fd, Port *port)
 			if (setsockopt(port->sock, SOL_SOCKET, SO_SNDBUF, (char *) &newopt,
 						   sizeof(newopt)) < 0)
 			{
-				elog(LOG, "setsockopt(%s) failed: %m", "SO_SNDBUF");
+				ereport(LOG,
+						(errmsg("setsockopt(%s) failed: %m", "SO_SNDBUF")));
 				return STATUS_ERROR;
 			}
 		}
@@ -1677,8 +1682,9 @@ pq_setkeepaliveswin32(Port *port, int idle, int interval)
 				 NULL)
 		!= 0)
 	{
-		elog(LOG, "WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
-			 WSAGetLastError());
+		ereport(LOG,
+				(errmsg("WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
+						WSAGetLastError())));
 		return STATUS_ERROR;
 	}
 	if (port->keepalives_idle != idle)
@@ -1708,7 +1714,8 @@ pq_getkeepalivesidle(Port *port)
 					   (char *) &port->default_keepalives_idle,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR);
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR)));
 			port->default_keepalives_idle = -1; /* don't know */
 		}
 #else							/* WIN32 */
@@ -1752,7 +1759,8 @@ pq_setkeepalivesidle(int idle, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, PG_TCP_KEEPALIVE_IDLE,
 				   (char *) &idle, sizeof(idle)) < 0)
 	{
-		elog(LOG, "setsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR);
+		ereport(LOG,
+				(errmsg("setsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR)));
 		return STATUS_ERROR;
 	}
 
@@ -1763,7 +1771,8 @@ pq_setkeepalivesidle(int idle, Port *port)
 #else
 	if (idle != 0)
 	{
-		elog(LOG, "setting the keepalive idle time is not supported");
+		ereport(LOG,
+				(errmsg("setting the keepalive idle time is not supported")));
 		return STATUS_ERROR;
 	}
 #endif
@@ -1790,7 +1799,8 @@ pq_getkeepalivesinterval(Port *port)
 					   (char *) &port->default_keepalives_interval,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(%s) failed: %m", "TCP_KEEPINTVL");
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", "TCP_KEEPINTVL")));
 			port->default_keepalives_interval = -1; /* don't know */
 		}
 #else
@@ -1833,7 +1843,8 @@ pq_setkeepalivesinterval(int interval, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPINTVL,
 				   (char *) &interval, sizeof(interval)) < 0)
 	{
-		elog(LOG, "setsockopt(%s) failed: %m", "TCP_KEEPINTVL");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) failed: %m", "TCP_KEEPINTVL")));
 		return STATUS_ERROR;
 	}
 
@@ -1844,7 +1855,8 @@ pq_setkeepalivesinterval(int interval, Port *port)
 #else
 	if (interval != 0)
 	{
-		elog(LOG, "setsockopt(%s) not supported", "TCP_KEEPINTVL");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) not supported", "TCP_KEEPINTVL")));
 		return STATUS_ERROR;
 	}
 #endif
@@ -1870,7 +1882,8 @@ pq_getkeepalivescount(Port *port)
 					   (char *) &port->default_keepalives_count,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(%s) failed: %m", "TCP_KEEPCNT");
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", "TCP_KEEPCNT")));
 			port->default_keepalives_count = -1;	/* don't know */
 		}
 	}
@@ -1908,7 +1921,8 @@ pq_setkeepalivescount(int count, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPCNT,
 				   (char *) &count, sizeof(count)) < 0)
 	{
-		elog(LOG, "setsockopt(%s) failed: %m", "TCP_KEEPCNT");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) failed: %m", "TCP_KEEPCNT")));
 		return STATUS_ERROR;
 	}
 
@@ -1916,7 +1930,8 @@ pq_setkeepalivescount(int count, Port *port)
 #else
 	if (count != 0)
 	{
-		elog(LOG, "setsockopt(%s) not supported", "TCP_KEEPCNT");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) not supported", "TCP_KEEPCNT")));
 		return STATUS_ERROR;
 	}
 #endif
@@ -1942,7 +1957,8 @@ pq_gettcpusertimeout(Port *port)
 					   (char *) &port->default_tcp_user_timeout,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(%s) failed: %m", "TCP_USER_TIMEOUT");
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", "TCP_USER_TIMEOUT")));
 			port->default_tcp_user_timeout = -1;	/* don't know */
 		}
 	}
@@ -1980,7 +1996,8 @@ pq_settcpusertimeout(int timeout, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, TCP_USER_TIMEOUT,
 				   (char *) &timeout, sizeof(timeout)) < 0)
 	{
-		elog(LOG, "setsockopt(%s) failed: %m", "TCP_USER_TIMEOUT");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) failed: %m", "TCP_USER_TIMEOUT")));
 		return STATUS_ERROR;
 	}
 
@@ -1988,7 +2005,8 @@ pq_settcpusertimeout(int timeout, Port *port)
 #else
 	if (timeout != 0)
 	{
-		elog(LOG, "setsockopt(%s) not supported", "TCP_USER_TIMEOUT");
+		ereport(LOG,
+				(errmsg("setsockopt(%s) not supported", "TCP_USER_TIMEOUT")));
 		return STATUS_ERROR;
 	}
 #endif
diff --git a/src/backend/optimizer/geqo/geqo_erx.c b/src/backend/optimizer/geqo/geqo_erx.c
index 3b92f420e0..fc9adaafa8 100644
--- a/src/backend/optimizer/geqo/geqo_erx.c
+++ b/src/backend/optimizer/geqo/geqo_erx.c
@@ -420,7 +420,8 @@ edge_failure(PlannerInfo *root, Gene *gene, int index, Edge *edge_table, int num
 			}
 		}
 
-		elog(LOG, "no edge found via random decision and total_edges == 4");
+		ereport(LOG,
+				(errmsg("no edge found via random decision and total_edges == 4")));
 	}
 	else if (remaining_edges != 0)
 	{
@@ -441,7 +442,8 @@ edge_failure(PlannerInfo *root, Gene *gene, int index, Edge *edge_table, int num
 			}
 		}
 
-		elog(LOG, "no edge found via random decision with remaining edges");
+		ereport(LOG,
+				(errmsg("no edge found via random decision with remaining edges")));
 	}
 
 	/*
@@ -459,7 +461,8 @@ edge_failure(PlannerInfo *root, Gene *gene, int index, Edge *edge_table, int num
 			if (edge_table[i].unused_edges >= 0)
 				return (Gene) i;
 
-		elog(LOG, "no edge found via looking for the last unused point");
+		ereport(LOG,
+				(errmsg("no edge found via looking for the last unused point")));
 	}
 
 
diff --git a/src/backend/port/posix_sema.c b/src/backend/port/posix_sema.c
index 277b82ca80..b6f57edfc8 100644
--- a/src/backend/port/posix_sema.c
+++ b/src/backend/port/posix_sema.c
@@ -149,11 +149,13 @@ PosixSemaphoreKill(sem_t *sem)
 #ifdef USE_NAMED_POSIX_SEMAPHORES
 	/* Got to use sem_close for named semaphores */
 	if (sem_close(sem) < 0)
-		elog(LOG, "sem_close failed: %m");
+		ereport(LOG,
+				(errmsg("sem_close() failed: %m")));
 #else
 	/* Got to use sem_destroy for unnamed semaphores */
 	if (sem_destroy(sem) < 0)
-		elog(LOG, "sem_destroy failed: %m");
+		ereport(LOG,
+				(errmsg("sem_destroy() failed: %m")));
 #endif
 }
 
@@ -306,7 +308,8 @@ PGSemaphoreReset(PGSemaphore sema)
 				break;			/* got it down to 0 */
 			if (errno == EINTR)
 				continue;		/* can this happen? */
-			elog(FATAL, "sem_trywait failed: %m");
+			ereport(FATAL,
+					(errmsg("sem_trywait() failed: %m")));
 		}
 	}
 }
@@ -328,7 +331,8 @@ PGSemaphoreLock(PGSemaphore sema)
 	} while (errStatus < 0 && errno == EINTR);
 
 	if (errStatus < 0)
-		elog(FATAL, "sem_wait failed: %m");
+		ereport(FATAL,
+				(errmsg("sem_wait() failed: %m")));
 }
 
 /*
@@ -353,7 +357,8 @@ PGSemaphoreUnlock(PGSemaphore sema)
 	} while (errStatus < 0 && errno == EINTR);
 
 	if (errStatus < 0)
-		elog(FATAL, "sem_post failed: %m");
+		ereport(FATAL,
+				(errmsg("sem_post() failed: %m")));
 }
 
 /*
@@ -381,7 +386,8 @@ PGSemaphoreTryLock(PGSemaphore sema)
 		if (errno == EAGAIN || errno == EDEADLK)
 			return false;		/* failed to lock it */
 		/* Otherwise we got trouble */
-		elog(FATAL, "sem_trywait failed: %m");
+		ereport(FATAL,
+				(errmsg("sem_trywait() failed: %m")));
 	}
 
 	return true;
diff --git a/src/backend/port/sysv_sema.c b/src/backend/port/sysv_sema.c
index 88c2862d58..181b5607c5 100644
--- a/src/backend/port/sysv_sema.c
+++ b/src/backend/port/sysv_sema.c
@@ -173,7 +173,8 @@ IpcSemaphoreKill(IpcSemaphoreId semId)
 	semun.val = 0;				/* unused, but keep compiler quiet */
 
 	if (semctl(semId, 0, IPC_RMID, semun) < 0)
-		elog(LOG, "semctl(%d, 0, IPC_RMID, ...) failed: %m", semId);
+		ereport(LOG,
+				(errmsg("semctl(%d, 0, IPC_RMID, ...) failed: %m", semId)));
 }
 
 /* Get the current value (semval) of the semaphore */
@@ -440,7 +441,8 @@ PGSemaphoreLock(PGSemaphore sema)
 	} while (errStatus < 0 && errno == EINTR);
 
 	if (errStatus < 0)
-		elog(FATAL, "semop(id=%d) failed: %m", sema->semId);
+		ereport(FATAL,
+				(errmsg("semop(id=%d) failed: %m", sema->semId)));
 }
 
 /*
@@ -470,7 +472,8 @@ PGSemaphoreUnlock(PGSemaphore sema)
 	} while (errStatus < 0 && errno == EINTR);
 
 	if (errStatus < 0)
-		elog(FATAL, "semop(id=%d) failed: %m", sema->semId);
+		ereport(FATAL,
+				(errmsg("semop(id=%d) failed: %m", sema->semId)));
 }
 
 /*
@@ -510,7 +513,8 @@ PGSemaphoreTryLock(PGSemaphore sema)
 			return false;		/* failed to lock it */
 #endif
 		/* Otherwise we got trouble */
-		elog(FATAL, "semop(id=%d) failed: %m", sema->semId);
+		ereport(FATAL,
+				(errmsg("semop(id=%d) failed: %m", sema->semId)));
 	}
 
 	return true;
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index 203555822d..6af1305dbb 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -198,8 +198,9 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
 				 * the original error.
 				 */
 				if (shmctl(shmid, IPC_RMID, NULL) < 0)
-					elog(LOG, "shmctl(%d, %d, 0) failed: %m",
-						 (int) shmid, IPC_RMID);
+					ereport(LOG,
+							(errmsg("shmctl(%d, %d, 0) failed: %m",
+									(int) shmid, IPC_RMID)));
 			}
 		}
 
@@ -248,8 +249,9 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
 	memAddress = shmat(shmid, requestedAddress, PG_SHMAT_FLAGS);
 
 	if (memAddress == (void *) -1)
-		elog(FATAL, "shmat(id=%d, addr=%p, flags=0x%x) failed: %m",
-			 shmid, requestedAddress, PG_SHMAT_FLAGS);
+		ereport(FATAL,
+				(errmsg("shmat(id=%d, addr=%p, flags=0x%x) failed: %m",
+						shmid, requestedAddress, PG_SHMAT_FLAGS)));
 
 	/* Register on-exit routine to detach new segment before deleting */
 	on_shmem_exit(IpcMemoryDetach, PointerGetDatum(memAddress));
@@ -280,7 +282,8 @@ IpcMemoryDetach(int status, Datum shmaddr)
 {
 	/* Detach System V shared memory block. */
 	if (shmdt(DatumGetPointer(shmaddr)) < 0)
-		elog(LOG, "shmdt(%p) failed: %m", DatumGetPointer(shmaddr));
+		ereport(LOG,
+				(errmsg("shmdt(%p) failed: %m", DatumGetPointer(shmaddr))));
 }
 
 /****************************************************************************/
@@ -291,8 +294,9 @@ static void
 IpcMemoryDelete(int status, Datum shmId)
 {
 	if (shmctl(DatumGetInt32(shmId), IPC_RMID, NULL) < 0)
-		elog(LOG, "shmctl(%d, %d, 0) failed: %m",
-			 DatumGetInt32(shmId), IPC_RMID);
+		ereport(LOG,
+				(errmsg("shmctl(%d, %d, 0) failed: %m",
+						DatumGetInt32(shmId), IPC_RMID)));
 }
 
 /*
@@ -314,7 +318,8 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
 
 	state = PGSharedMemoryAttach((IpcMemoryId) id2, NULL, &memAddress);
 	if (memAddress && shmdt(memAddress) < 0)
-		elog(LOG, "shmdt(%p) failed: %m", memAddress);
+		ereport(LOG,
+				(errmsg("shmdt(%p) failed: %m", memAddress)));
 	switch (state)
 	{
 		case SHMSTATE_ENOENT:
@@ -629,8 +634,9 @@ AnonymousShmemDetach(int status, Datum arg)
 	if (AnonymousShmem != NULL)
 	{
 		if (munmap(AnonymousShmem, AnonymousShmemSize) < 0)
-			elog(LOG, "munmap(%p, %zu) failed: %m",
-				 AnonymousShmem, AnonymousShmemSize);
+			ereport(LOG,
+					(errmsg("munmap(%p, %zu) failed: %m",
+							AnonymousShmem, AnonymousShmemSize)));
 		AnonymousShmem = NULL;
 	}
 }
@@ -747,10 +753,10 @@ PGSharedMemoryCreate(Size size,
 				 * InternalIpcMemoryCreate().  Moments earlier, we would have
 				 * seen SHMSTATE_FOREIGN.  Try that same ID again.
 				 */
-				elog(LOG,
-					 "shared memory block (key %lu, ID %lu) deleted during startup",
-					 (unsigned long) NextShmemSegID,
-					 (unsigned long) shmid);
+				ereport(LOG,
+						(errmsg("shared memory block (key %lu, ID %lu) deleted during startup",
+								(unsigned long) NextShmemSegID,
+								(unsigned long) shmid)));
 				break;
 			case SHMSTATE_FOREIGN:
 				NextShmemSegID++;
@@ -775,7 +781,8 @@ PGSharedMemoryCreate(Size size,
 		}
 
 		if (oldhdr && shmdt(oldhdr) < 0)
-			elog(LOG, "shmdt(%p) failed: %m", oldhdr);
+			ereport(LOG,
+					(errmsg("shmdt(%p) failed: %m", oldhdr)));
 	}
 
 	/* Initialize new segment. */
@@ -849,11 +856,13 @@ PGSharedMemoryReAttach(void)
 	else
 		state = PGSharedMemoryAttach(shmid, UsedShmemSegAddr, &hdr);
 	if (state != SHMSTATE_ATTACHED)
-		elog(FATAL, "could not reattach to shared memory (key=%d, addr=%p): %m",
-			 (int) UsedShmemSegID, UsedShmemSegAddr);
+		ereport(FATAL,
+				(errmsg("could not reattach to shared memory (key=%d, addr=%p): %m",
+						(int) UsedShmemSegID, UsedShmemSegAddr)));
 	if (hdr != origUsedShmemSegAddr)
-		elog(FATAL, "reattaching to shared memory returned unexpected address (got %p, expected %p)",
-			 hdr, origUsedShmemSegAddr);
+		ereport(FATAL,
+				(errmsg("reattaching to shared memory returned unexpected address (got %p, expected %p)",
+						hdr, origUsedShmemSegAddr)));
 	dsm_set_control_handle(hdr->dsm_control);
 
 	UsedShmemSegAddr = hdr;		/* probably redundant */
@@ -916,15 +925,17 @@ PGSharedMemoryDetach(void)
 			&& shmdt(NULL) < 0
 #endif
 			)
-			elog(LOG, "shmdt(%p) failed: %m", UsedShmemSegAddr);
+			ereport(LOG,
+					(errmsg("shmdt(%p) failed: %m", UsedShmemSegAddr)));
 		UsedShmemSegAddr = NULL;
 	}
 
 	if (AnonymousShmem != NULL)
 	{
 		if (munmap(AnonymousShmem, AnonymousShmemSize) < 0)
-			elog(LOG, "munmap(%p, %zu) failed: %m",
-				 AnonymousShmem, AnonymousShmemSize);
+			ereport(LOG,
+					(errmsg("munmap(%p, %zu) failed: %m",
+							AnonymousShmem, AnonymousShmemSize)));
 		AnonymousShmem = NULL;
 	}
 }
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 30b07303ff..3924c2250d 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -69,18 +69,21 @@ GetSharedMemName(void)
 
 	bufsize = GetFullPathName(DataDir, 0, NULL, NULL);
 	if (bufsize == 0)
-		elog(FATAL, "could not get size for full pathname of datadir %s: error code %lu",
-			 DataDir, GetLastError());
+		ereport(FATAL,
+				(errmsg("could not get size for full path name of data directory \"%s\": error code %lu",
+						DataDir, GetLastError())));
 
 	retptr = malloc(bufsize + 18);	/* 18 for Global\PostgreSQL: */
 	if (retptr == NULL)
-		elog(FATAL, "could not allocate memory for shared memory name");
+		ereport(FATAL,
+				(errmsg("could not allocate memory for shared memory name")));
 
 	strcpy(retptr, "Global\\PostgreSQL:");
 	r = GetFullPathName(DataDir, bufsize, retptr + 18, NULL);
 	if (r == 0 || r > bufsize)
-		elog(FATAL, "could not generate full pathname for datadir %s: error code %lu",
-			 DataDir, GetLastError());
+		ereport(FATAL,
+				(errmsg("could not generate full path name for data directory \"%s\": error code %lu",
+						DataDir, GetLastError())));
 
 	/*
 	 * XXX: Intentionally overwriting the Global\ part here. This was not the
@@ -343,7 +346,8 @@ PGSharedMemoryCreate(Size size,
 	 * care.
 	 */
 	if (!CloseHandle(hmap))
-		elog(LOG, "could not close handle to shared memory: error code %lu", GetLastError());
+		ereport(LOG,
+				(errmsg("could not close handle to shared memory: error code %lu", GetLastError())));
 
 
 	/*
@@ -414,18 +418,22 @@ PGSharedMemoryReAttach(void)
 		elog(FATAL, "failed to release reserved memory region (addr=%p): error code %lu",
 			 ShmemProtectiveRegion, GetLastError());
 	if (VirtualFree(UsedShmemSegAddr, 0, MEM_RELEASE) == 0)
-		elog(FATAL, "failed to release reserved memory region (addr=%p): error code %lu",
-			 UsedShmemSegAddr, GetLastError());
+		ereport(FATAL,
+				(errmsg("could not release reserved memory region (addr=%p): error code %lu",
+						UsedShmemSegAddr, GetLastError())));
 
 	hdr = (PGShmemHeader *) MapViewOfFileEx(UsedShmemSegID, FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, 0, UsedShmemSegAddr);
 	if (!hdr)
-		elog(FATAL, "could not reattach to shared memory (key=%p, addr=%p): error code %lu",
-			 UsedShmemSegID, UsedShmemSegAddr, GetLastError());
+		ereport(FATAL,
+				(errmsg("could not reattach to shared memory (key=%p, addr=%p): error code %lu",
+						UsedShmemSegID, UsedShmemSegAddr, GetLastError())));
 	if (hdr != origUsedShmemSegAddr)
-		elog(FATAL, "reattaching to shared memory returned unexpected address (got %p, expected %p)",
-			 hdr, origUsedShmemSegAddr);
+		ereport(FATAL,
+				(errmsg("reattaching to shared memory returned unexpected address (got %p, expected %p)",
+						hdr, origUsedShmemSegAddr)));
 	if (hdr->magic != PGShmemMagic)
-		elog(FATAL, "reattaching to shared memory returned non-PostgreSQL memory");
+		ereport(FATAL,
+				(errmsg("reattaching to shared memory returned non-PostgreSQL memory")));
 	dsm_set_control_handle(hdr->dsm_control);
 
 	UsedShmemSegAddr = hdr;		/* probably redundant */
@@ -488,8 +496,9 @@ PGSharedMemoryDetach(void)
 	if (ShmemProtectiveRegion != NULL)
 	{
 		if (VirtualFree(ShmemProtectiveRegion, 0, MEM_RELEASE) == 0)
-			elog(LOG, "failed to release reserved memory region (addr=%p): error code %lu",
-				 ShmemProtectiveRegion, GetLastError());
+			ereport(LOG,
+					(errmsg("could not release reserved memory region (addr=%p): error code %lu",
+							ShmemProtectiveRegion, GetLastError())));
 
 		ShmemProtectiveRegion = NULL;
 	}
@@ -498,8 +507,9 @@ PGSharedMemoryDetach(void)
 	if (UsedShmemSegAddr != NULL)
 	{
 		if (!UnmapViewOfFile(UsedShmemSegAddr))
-			elog(LOG, "could not unmap view of shared memory: error code %lu",
-				 GetLastError());
+			ereport(LOG,
+					(errmsg("could not unmap view of shared memory: error code %lu",
+							GetLastError())));
 
 		UsedShmemSegAddr = NULL;
 	}
@@ -508,8 +518,9 @@ PGSharedMemoryDetach(void)
 	if (UsedShmemSegID != INVALID_HANDLE_VALUE)
 	{
 		if (!CloseHandle(UsedShmemSegID))
-			elog(LOG, "could not close handle to shared memory: error code %lu",
-				 GetLastError());
+			ereport(LOG,
+					(errmsg("could not close handle to shared memory: error code %lu",
+							GetLastError())));
 
 		UsedShmemSegID = INVALID_HANDLE_VALUE;
 	}
@@ -544,7 +555,7 @@ pgwin32_SharedMemoryDelete(int status, Datum shmId)
  * be given different address ranges that don't conflict.
  *
  * NOTE! This function executes in the postmaster, and should for this
- * reason not use elog(FATAL) since that would take down the postmaster.
+ * reason not use ereport(FATAL) since that would take down the postmaster.
  */
 int
 pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
@@ -562,8 +573,9 @@ pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
 	if (address == NULL)
 	{
 		/* Don't use FATAL since we're running in the postmaster */
-		elog(LOG, "could not reserve shared memory region (addr=%p) for child %p: error code %lu",
-			 ShmemProtectiveRegion, hChild, GetLastError());
+		ereport(LOG,
+				(errmsg("could not reserve shared memory region (addr=%p) for child %p: error code %lu",
+						ShmemProtectiveRegion, hChild, GetLastError())));
 		return false;
 	}
 	if (address != ShmemProtectiveRegion)
@@ -574,8 +586,9 @@ pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
 		 *
 		 * Don't use FATAL since we're running in the postmaster.
 		 */
-		elog(LOG, "reserved shared memory region got incorrect address %p, expected %p",
-			 address, ShmemProtectiveRegion);
+		ereport(LOG,
+				(errmsg("reserved shared memory region got incorrect address %p, expected %p",
+						address, ShmemProtectiveRegion)));
 		return false;
 	}
 
@@ -584,14 +597,16 @@ pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
 							 MEM_RESERVE, PAGE_READWRITE);
 	if (address == NULL)
 	{
-		elog(LOG, "could not reserve shared memory region (addr=%p) for child %p: error code %lu",
-			 UsedShmemSegAddr, hChild, GetLastError());
+		ereport(LOG,
+				(errmsg("could not reserve shared memory region (addr=%p) for child %p: error code %lu",
+						UsedShmemSegAddr, hChild, GetLastError())));
 		return false;
 	}
 	if (address != UsedShmemSegAddr)
 	{
-		elog(LOG, "reserved shared memory region got incorrect address %p, expected %p",
-			 address, UsedShmemSegAddr);
+		ereport(LOG,
+				(errmsg("reserved shared memory region got incorrect address %p, expected %p",
+						address, UsedShmemSegAddr)));
 		return false;
 	}
 
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 5a9a0e3435..7ef6259eb5 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -250,10 +250,10 @@ BackgroundWorkerStateChange(void)
 	 */
 	if (max_worker_processes != BackgroundWorkerData->total_slots)
 	{
-		elog(LOG,
-			 "inconsistent background worker state (max_worker_processes=%d, total_slots=%d",
-			 max_worker_processes,
-			 BackgroundWorkerData->total_slots);
+		ereport(LOG,
+				(errmsg("inconsistent background worker state (max_worker_processes=%d, total_slots=%d)",
+						max_worker_processes,
+						BackgroundWorkerData->total_slots)));
 		return;
 	}
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9bad14981b..048c6d6830 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -636,7 +636,8 @@ pgstat_init(void)
 		if (getsockopt(pgStatSock, SOL_SOCKET, SO_RCVBUF,
 					   (char *) &old_rcvbuf, &rcvbufsize) < 0)
 		{
-			elog(LOG, "getsockopt(SO_RCVBUF) failed: %m");
+			ereport(LOG,
+					(errmsg("getsockopt(%s) failed: %m", "SO_RCVBUF")));
 			/* if we can't get existing size, always try to set it */
 			old_rcvbuf = 0;
 		}
@@ -646,7 +647,8 @@ pgstat_init(void)
 		{
 			if (setsockopt(pgStatSock, SOL_SOCKET, SO_RCVBUF,
 						   (char *) &new_rcvbuf, sizeof(new_rcvbuf)) < 0)
-				elog(LOG, "setsockopt(SO_RCVBUF) failed: %m");
+				ereport(LOG,
+						(errmsg("setsockopt(%s) failed: %m", "SO_RCVBUF")));
 		}
 	}
 
@@ -5264,7 +5266,8 @@ get_dbstat_filename(bool permanent, bool tempname, Oid databaseid,
 					   databaseid,
 					   tempname ? "tmp" : "stat");
 	if (printed >= len)
-		elog(ERROR, "overlength pgstat path");
+		ereport(ERROR,
+				(errmsg("pgstat path too long")));
 }
 
 /* ----------
@@ -6108,8 +6111,9 @@ backend_read_statsfile(void)
 				/* Copy because timestamptz_to_str returns a static buffer */
 				filetime = pstrdup(timestamptz_to_str(file_ts));
 				mytime = pstrdup(timestamptz_to_str(cur_ts));
-				elog(LOG, "stats collector's time %s is later than backend local time %s",
-					 filetime, mytime);
+				ereport(LOG,
+						(errmsg("statistics collector's time %s is later than backend local time %s",
+								filetime, mytime)));
 				pfree(filetime);
 				pfree(mytime);
 			}
@@ -6251,9 +6255,9 @@ pgstat_recv_inquiry(PgStat_MsgInquiry *msg, int len)
 			/* Copy because timestamptz_to_str returns a static buffer */
 			writetime = pstrdup(timestamptz_to_str(dbentry->stats_timestamp));
 			mytime = pstrdup(timestamptz_to_str(cur_ts));
-			elog(LOG,
-				 "stats_timestamp %s is later than collector's time %s for database %u",
-				 writetime, mytime, dbentry->databaseid);
+			ereport(LOG,
+					(errmsg("stats_timestamp %s is later than collector's time %s for database %u",
+							writetime, mytime, dbentry->databaseid)));
 			pfree(writetime);
 			pfree(mytime);
 		}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b7799ed1d2..5d09822c81 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1207,8 +1207,9 @@ PostmasterMain(int argc, char *argv[])
 								 NULL,
 								 NULL);
 		if (err != kDNSServiceErr_NoError)
-			elog(LOG, "DNSServiceRegister() failed: error code %ld",
-				 (long) err);
+			ereport(LOG,
+					(errmsg("DNSServiceRegister() failed: error code %ld",
+							(long) err)));
 
 		/*
 		 * We don't bother to read the mDNS daemon's reply, and we expect that
@@ -1466,7 +1467,8 @@ getInstallationPaths(const char *argv0)
 
 	/* Locate the postgres executable itself */
 	if (find_my_exec(argv0, my_exec_path) < 0)
-		elog(FATAL, "%s: could not locate my own executable path", argv0);
+		ereport(FATAL,
+				(errmsg("%s: could not locate my own executable path", argv0)));
 
 #ifdef EXEC_BACKEND
 	/* Locate executable backend before we change working directory */
@@ -4674,16 +4676,18 @@ internal_forkexec(int argc, char *argv[], Port *port)
 									NULL);
 	if (paramHandle == INVALID_HANDLE_VALUE)
 	{
-		elog(LOG, "could not create backend parameter file mapping: error code %lu",
-			 GetLastError());
+		ereport(LOG,
+				(errmsg("could not create backend parameter file mapping: error code %lu",
+						GetLastError())));
 		return -1;
 	}
 
 	param = MapViewOfFile(paramHandle, FILE_MAP_WRITE, 0, 0, sizeof(BackendParameters));
 	if (!param)
 	{
-		elog(LOG, "could not map backend parameter memory: error code %lu",
-			 GetLastError());
+		ereport(LOG,
+				(errmsg("could not map backend parameter memory: error code %lu",
+						GetLastError())));
 		CloseHandle(paramHandle);
 		return -1;
 	}
@@ -4708,7 +4712,8 @@ internal_forkexec(int argc, char *argv[], Port *port)
 	}
 	if (cmdLine[sizeof(cmdLine) - 2] != '\0')
 	{
-		elog(LOG, "subprocess command line too long");
+		ereport(LOG,
+				(errmsg("subprocess command line too long")));
 		UnmapViewOfFile(param);
 		CloseHandle(paramHandle);
 		return -1;
@@ -4725,8 +4730,9 @@ internal_forkexec(int argc, char *argv[], Port *port)
 	if (!CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, CREATE_SUSPENDED,
 					   NULL, NULL, &si, &pi))
 	{
-		elog(LOG, "CreateProcess call failed: %m (error code %lu)",
-			 GetLastError());
+		ereport(LOG,
+				(errmsg("CreateProcess() call failed: %m (error code %lu)",
+						GetLastError())));
 		UnmapViewOfFile(param);
 		CloseHandle(paramHandle);
 		return -1;
@@ -4751,11 +4757,13 @@ internal_forkexec(int argc, char *argv[], Port *port)
 
 	/* Drop the parameter shared memory that is now inherited to the backend */
 	if (!UnmapViewOfFile(param))
-		elog(LOG, "could not unmap view of backend parameter file: error code %lu",
-			 GetLastError());
+		ereport(LOG,
+				(errmsg("could not unmap view of backend parameter file: error code %lu",
+						GetLastError())));
 	if (!CloseHandle(paramHandle))
-		elog(LOG, "could not close handle to backend parameter file: error code %lu",
-			 GetLastError());
+		ereport(LOG,
+				(errmsg("could not close handle to backend parameter file: error code %lu",
+						GetLastError())));
 
 	/*
 	 * Reserve the memory region used by our main shared memory segment before
@@ -5639,7 +5647,9 @@ CreateOptsFile(int argc, char *argv[], char *fullprogname)
 
 	if ((fp = fopen(OPTS_FILE, "w")) == NULL)
 	{
-		elog(LOG, "could not create file \"%s\": %m", OPTS_FILE);
+		ereport(LOG,
+				(errcode_for_file_access(),
+				 errmsg("could not create file \"%s\": %m", OPTS_FILE)));
 		return false;
 	}
 
@@ -5650,7 +5660,9 @@ CreateOptsFile(int argc, char *argv[], char *fullprogname)
 
 	if (fclose(fp))
 	{
-		elog(LOG, "could not write file \"%s\": %m", OPTS_FILE);
+		ereport(LOG,
+				(errcode_for_file_access(),
+				 errmsg("could not write file \"%s\": %m", OPTS_FILE)));
 		return false;
 	}
 
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 1b220315df..15ab8e7204 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -769,10 +769,11 @@ StartupReplicationOrigin(void)
 		replication_states[last_state].remote_lsn = disk_state.remote_lsn;
 		last_state++;
 
-		elog(LOG, "recovered replication state of node %u to %X/%X",
-			 disk_state.roident,
-			 (uint32) (disk_state.remote_lsn >> 32),
-			 (uint32) disk_state.remote_lsn);
+		ereport(LOG,
+				(errmsg("recovered replication state of node %u to %X/%X",
+						disk_state.roident,
+						(uint32) (disk_state.remote_lsn >> 32),
+						(uint32) disk_state.remote_lsn)));
 	}
 
 	/* now check checksum */
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 88004c6fae..c24c0ff69f 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1914,7 +1914,8 @@ FileClose(File file)
 
 		/* in any case do the unlink */
 		if (unlink(vfdP->fileName))
-			elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
+			ereport(LOG,
+					(errmsg("could not unlink file \"%s\": %m", vfdP->fileName)));
 
 		/* and last report the stat results */
 		if (stat_errno == 0)
@@ -1922,7 +1923,8 @@ FileClose(File file)
 		else
 		{
 			errno = stat_errno;
-			elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
+			ereport(LOG,
+					(errmsg("could not stat file \"%s\": %m", vfdP->fileName)));
 		}
 	}
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 02d2d267b5..635d91d50a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11533,7 +11533,7 @@ assign_tcp_keepalives_idle(int newval, void *extra)
 	 * once we set it we might fail to unset it.  So there seems little point
 	 * in fully implementing the check-then-assign GUC API for these
 	 * variables.  Instead we just do the assignment on demand.  pqcomm.c
-	 * reports any problems via elog(LOG).
+	 * reports any problems via ereport(LOG).
 	 *
 	 * This approach means that the GUC value might have little to do with the
 	 * actual kernel value, so we use a show_hook that retrieves the kernel
-- 
2.29.2

#2Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Peter Eisentraut (#1)
Re: convert elog(LOG) calls to ereport

On 2020-Dec-02, Peter Eisentraut wrote:

There are a number of elog(LOG) calls that appear to be user-facing, so they
should be ereport()s. This patch changes them. There are more elog(LOG)
calls remaining, but they all appear to be some kind of debugging support.
Also, I changed a few elog(FATAL)s that were nearby, but I didn't
specifically look for them.

-		elog(LOG, "WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
-			 WSAGetLastError());
+		ereport(LOG,
+				(errmsg("WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
+						WSAGetLastError())));

Please take the opportunity to move the flag name out of the message in
this one, also. I do wonder if it'd be a good idea to move the syscall
name itself out of the message, too; that would reduce the number of
messages to translate 50x to just "%s(%s) failed: %m" instead of one
message per distinct syscall.

Should fd.c messages do errcode_for_file_access() like elsewhere?

Overall, it looks good to me.

Thanks

#3Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#2)
Re: convert elog(LOG) calls to ereport

On Wed, Dec 02, 2020 at 11:04:45AM -0300, Alvaro Herrera wrote:

Please take the opportunity to move the flag name out of the message in
this one, also. I do wonder if it'd be a good idea to move the syscall
name itself out of the message, too; that would reduce the number of
messages to translate 50x to just "%s(%s) failed: %m" instead of one
message per distinct syscall.

+1.

+   else
+       ereport(LOG,
+               (errmsg("checkpoint starting:%s%s%s%s%s%s%s%s",

Would it be better to add a note for translators here, in short that
all those %s are options related to checkpoint/restartpoints?

The ones in geqo_erx.c look like debugging information, and the ones
in win32_shmem.c for segment creation are code paths only used by the
postmaster.
--
Michael

#4Noah Misch
noah@leadboat.com
In reply to: Peter Eisentraut (#1)
Re: convert elog(LOG) calls to ereport

On Wed, Dec 02, 2020 at 02:26:24PM +0100, Peter Eisentraut wrote:

There are a number of elog(LOG) calls that appear to be user-facing, so they
should be ereport()s.

@@ -8591,25 +8604,46 @@ LogCheckpointEnd(bool restartpoint)
CheckpointStats.ckpt_sync_rels;
average_msecs = (long) ((average_sync_time + 999) / 1000);

- elog(LOG, "%s complete: wrote %d buffers (%.1f%%); "
- "%d WAL file(s) added, %d removed, %d recycled; "
- "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
- "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB",

+1 to this change.

@@ -1763,7 +1771,8 @@ pq_setkeepalivesidle(int idle, Port *port)
#else
if (idle != 0)
{
-		elog(LOG, "setting the keepalive idle time is not supported");
+		ereport(LOG,
+				(errmsg("setting the keepalive idle time is not supported")));

+1

--- a/src/backend/optimizer/geqo/geqo_erx.c
+++ b/src/backend/optimizer/geqo/geqo_erx.c
@@ -420,7 +420,8 @@ edge_failure(PlannerInfo *root, Gene *gene, int index, Edge *edge_table, int num
}
}
-		elog(LOG, "no edge found via random decision and total_edges == 4");
+		ereport(LOG,
+				(errmsg("no edge found via random decision and total_edges == 4")));

The user can't act upon this without reading the source code. This and the
other messages proposed in this file are better as elog().

@@ -343,7 +346,8 @@ PGSharedMemoryCreate(Size size,
* care.
*/
if (!CloseHandle(hmap))
-		elog(LOG, "could not close handle to shared memory: error code %lu", GetLastError());
+		ereport(LOG,
+				(errmsg("could not close handle to shared memory: error code %lu", GetLastError())));

The numerous messages proposed in src/backend/port/ files are can't-happen
events, and there's little a DBA can do without reading the source code.
They're better as elog().

@@ -6108,8 +6111,9 @@ backend_read_statsfile(void)
/* Copy because timestamptz_to_str returns a static buffer */
filetime = pstrdup(timestamptz_to_str(file_ts));
mytime = pstrdup(timestamptz_to_str(cur_ts));
-				elog(LOG, "stats collector's time %s is later than backend local time %s",
-					 filetime, mytime);
+				ereport(LOG,
+						(errmsg("statistics collector's time %s is later than backend local time %s",
+								filetime, mytime)));

+1

@@ -769,10 +769,11 @@ StartupReplicationOrigin(void)
replication_states[last_state].remote_lsn = disk_state.remote_lsn;
last_state++;

-		elog(LOG, "recovered replication state of node %u to %X/%X",
-			 disk_state.roident,
-			 (uint32) (disk_state.remote_lsn >> 32),
-			 (uint32) disk_state.remote_lsn);
+		ereport(LOG,
+				(errmsg("recovered replication state of node %u to %X/%X",
+						disk_state.roident,
+						(uint32) (disk_state.remote_lsn >> 32),
+						(uint32) disk_state.remote_lsn)));

+1

@@ -1914,7 +1914,8 @@ FileClose(File file)

/* in any case do the unlink */
if (unlink(vfdP->fileName))
-			elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
+			ereport(LOG,
+					(errmsg("could not unlink file \"%s\": %m", vfdP->fileName)));

+1

/* and last report the stat results */
if (stat_errno == 0)
@@ -1922,7 +1923,8 @@ FileClose(File file)
else
{
errno = stat_errno;
-			elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
+			ereport(LOG,
+					(errmsg("could not stat file \"%s\": %m", vfdP->fileName)));

+1

For the changes I didn't mention explicitly (most of them), I'm -0.5. Many of
them "can't happen", use source code terms of art, and/or breach guideline
"Avoid mentioning called function names, either; instead say what the code was
trying to do" (https://www.postgresql.org/docs/devel/error-style-guide.html).

#5Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Alvaro Herrera (#2)
Re: convert elog(LOG) calls to ereport

On 2020-12-02 15:04, Alvaro Herrera wrote:

-		elog(LOG, "WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
-			 WSAGetLastError());
+		ereport(LOG,
+				(errmsg("WSAIoctl(SIO_KEEPALIVE_VALS) failed: %ui",
+						WSAGetLastError())));

Please take the opportunity to move the flag name out of the message in
this one, also.

done

I do wonder if it'd be a good idea to move the syscall
name itself out of the message, too; that would reduce the number of
messages to translate 50x to just "%s(%s) failed: %m" instead of one
message per distinct syscall.

Seems useful, but perhaps as a separate project.

Should fd.c messages do errcode_for_file_access() like elsewhere?

yes, done

#6Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Michael Paquier (#3)
Re: convert elog(LOG) calls to ereport

On 2020-12-03 08:02, Michael Paquier wrote:

+   else
+       ereport(LOG,
+               (errmsg("checkpoint starting:%s%s%s%s%s%s%s%s",

Would it be better to add a note for translators here, in short that
all those %s are options related to checkpoint/restartpoints?

done

The ones in geqo_erx.c look like debugging information, and the ones
in win32_shmem.c for segment creation are code paths only used by the
postmaster.

I dialed those back a bit.

#7Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Noah Misch (#4)
Re: convert elog(LOG) calls to ereport

On 2020-12-03 08:55, Noah Misch wrote:

For the changes I didn't mention explicitly (most of them), I'm -0.5. Many of
them "can't happen", use source code terms of art, and/or breach guideline
"Avoid mentioning called function names, either; instead say what the code was
trying to do" (https://www.postgresql.org/docs/devel/error-style-guide.html).

Thanks for the detailed feedback. I dialed back some of the changes
based on your feedback.

#8Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#5)
Re: convert elog(LOG) calls to ereport

On Fri, Dec 04, 2020 at 02:34:26PM +0100, Peter Eisentraut wrote:

On 2020-12-02 15:04, Alvaro Herrera wrote:

I do wonder if it'd be a good idea to move the syscall
name itself out of the message, too; that would reduce the number of
messages to translate 50x to just "%s(%s) failed: %m" instead of one
message per distinct syscall.

Seems useful, but perhaps as a separate project.

-       elog(LOG, "getsockname() failed: %m");
+       ereport(LOG,
+               (errmsg("getsockname() failed: %m")));
FWIW, I disagree with the approach taken by eb93f3a.  As of HEAD, it
is now required to translate all those strings.  I think that it would
have been better to remove the function names from all those error
messages and not require the same pattern to be translated N times.
--
Michael
#9Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Michael Paquier (#8)
Re: convert elog(LOG) calls to ereport

On 05.12.20 03:22, Michael Paquier wrote:

On Fri, Dec 04, 2020 at 02:34:26PM +0100, Peter Eisentraut wrote:

On 2020-12-02 15:04, Alvaro Herrera wrote:

I do wonder if it'd be a good idea to move the syscall
name itself out of the message, too; that would reduce the number of
messages to translate 50x to just "%s(%s) failed: %m" instead of one
message per distinct syscall.

Seems useful, but perhaps as a separate project.

-       elog(LOG, "getsockname() failed: %m");
+       ereport(LOG,
+               (errmsg("getsockname() failed: %m")));
FWIW, I disagree with the approach taken by eb93f3a.  As of HEAD, it
is now required to translate all those strings.  I think that it would
have been better to remove the function names from all those error
messages and not require the same pattern to be translated N times.

I made another pass across this and implemented the requested change.