On Sat, Nov 15, 2014 at 3:42 AM, Andres Freund <[email protected]> wrote:
> On 2014-11-15 03:25:16 +0900, Fujii Masao wrote:
>> On Fri, Nov 14, 2014 at 7:22 PM, <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
