On Sat, Nov 15, 2014 at 3:42 AM, Andres Freund <and...@2ndquadrant.com> wrote:
> On 2014-11-15 03:25:16 +0900, Fujii Masao wrote:
>> On Fri, Nov 14, 2014 at 7:22 PM,  <furu...@pm.nttdata.co.jp> wrote:
>> > "pg_ctl stop" does't work propley, if --slot option is specified when WAL 
>> > is flushed only it has switched.
>> > These processes still continue even after the posmaster 
>> > failed:pg_receivexlog, walsender and logger.
>>
>> I could reproduce this problem. At normal shutdown, walsender keeps waiting
>> for the last WAL record to be replicated and flushed in pg_receivexlog. But
>> pg_receivexlog issues sync command only when WAL file is switched. Thus,
>> since pg_receivexlog may never flush the last WAL record, walsender may have
>> to keep waiting infinitely.
>
> Right.
It is surprising that nobody complained about that before,
pg_receivexlog has been released two years ago.

>> pg_recvlogical handles this problem by calling fsync() when it receives the
>> request of immediate reply from the server. That is, at shutdown, walsender
>> sends the request, pg_receivexlog receives it, flushes the last WAL record,
>> and sends the flush location back to the server. Since walsender can see that
>> the last WAL record is successfully flushed in pg_receivexlog, it can
>> exit cleanly.
>>
>> One idea to the problem is to introduce the same logic as pg_recvlogical has,
>> to pg_receivexlog. Thought?
>
> Sounds sane to me. Are you looking into doing that?
Yep, sounds a good thing to do if master requested answer from the
client in the keepalive message. Something like the patch attached
would make the deal.
-- 
Michael
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index c6c90fb..b93d52b 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -149,6 +149,34 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir,
 }
 
 /*
+ * Flush the current WAL file to disk and update the last WAL flush
+ * positions as well as the last fsync timestamp. On failure, print
+ * a message to stderr and return false, otherwise return true.
+ */
+static bool
+fsync_walfile(XLogRecPtr blockpos, int64 timestamp)
+{
+	/* nothing to do if no WAL file open */
+	if (walfile == -1)
+		return true;
+
+	/* nothing to do if data has already been flushed */
+	if (blockpos <= lastFlushPosition)
+		return true;
+
+	if (fsync(walfile) != 0)
+	{
+		fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
+				progname, current_walfile_name, strerror(errno));
+		return false;
+	}
+
+	lastFlushPosition = blockpos;
+	last_fsync = timestamp;
+	return true;
+}
+
+/*
  * Close the current WAL file (if open), and rename it to the correct
  * filename if it's complete. On failure, prints an error message to stderr
  * and returns false, otherwise returns true.
@@ -787,21 +815,12 @@ HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
 		 * If fsync_interval has elapsed since last WAL flush and we've written
 		 * some WAL data, flush them to disk.
 		 */
-		if (lastFlushPosition < blockpos &&
-			walfile != -1 &&
-			((fsync_interval > 0 &&
-			  feTimestampDifferenceExceeds(last_fsync, now, fsync_interval)) ||
-			 fsync_interval < 0))
+		if ((fsync_interval > 0 &&
+			 feTimestampDifferenceExceeds(last_fsync, now, fsync_interval)) ||
+			 fsync_interval < 0)
 		{
-			if (fsync(walfile) != 0)
-			{
-				fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
-						progname, current_walfile_name, strerror(errno));
+			if (!fsync_walfile(blockpos, now))
 				goto error;
-			}
-
-			lastFlushPosition = blockpos;
-			last_fsync = now;
 		}
 
 		/*
@@ -999,7 +1018,6 @@ ProcessKeepaliveMsg(PGconn *conn, char *copybuf, int len,
 {
 	int			pos;
 	bool		replyRequested;
-	int64		now;
 
 	/*
 	 * Parse the keepalive message, enclosed in the CopyData message.
@@ -1021,7 +1039,12 @@ ProcessKeepaliveMsg(PGconn *conn, char *copybuf, int len,
 	/* If the server requested an immediate reply, send one. */
 	if (replyRequested && still_sending)
 	{
-		now = feGetCurrentTimestamp();
+		int64		now = feGetCurrentTimestamp();
+
+		/* fsync data, so a fresh flush position is sent */
+		if (!fsync_walfile(blockpos, now))
+			return false;
+
 		if (!sendFeedback(conn, blockpos, now, false))
 			return false;
 		*last_status = now;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to