On Tue, Feb 7, 2012 at 7:06 PM, Magnus Hagander <mag...@hagander.net> wrote:
> On Tue, Feb 7, 2012 at 10:55, Heikki Linnakangas
> <heikki.linnakan...@enterprisedb.com> wrote:
>> On 07.02.2012 11:35, Magnus Hagander wrote:
>>>
>>> On Tue, Feb 7, 2012 at 10:31, Heikki Linnakangas
>>> <heikki.linnakan...@enterprisedb.com> wrote:
>>>>
>>>> So, --statusint needs to be in milliseconds. And while we're at it, how
Attached patch does that and fixes the problem caused under
--disable-integer-datetimes.
>>>>
>>>> difficult would be to ask the server for the current value of
>>>> replication_timeout, and set --statusint automatically based on that? Or
>>>> perhaps mark replication_timeout as GUC_REPORT. It is rather fiddly that
>>>> depending on a server setting, you need to pass an option in the client
>>>> or
>>>> it will just silently fail with no indication of what the problem is.
>>>
>>>
>>> We can't really ask for it easily, since we're on a replication
>>> connection. Unless we add that to the walsender grammar, but that
>>> would make it impossible to use the receivexlog stuff against a 9.1
>>> server (which I think still works, though I haven't tested it in a
>>> while).
>>
>>
>> You could put a version-check there, and only send the command if connected
>> to a 9.2 server.
>
> True..
>
>
>>> Do we have a facility to make it a GUC_REPORT but only for walsender
>>> connections?
>>
>>
>> There's no such facility at the moment.
>>
>>
>>> It seems like a very unnecessary thing to have it sent out over every
>>> single connection, since it would only be useful in a very small
>>> subset of them.
>>
>>
>> True, and conversely, many of the current GUC_REPORT settings don't apply to
>> replication clients at all. Like standard_conforming_strings and DateStyle.
>
> Right. But it's less important there, since the replication
> connections will in any reasonable configuration be only a tiny part
> of the connections.
>
>
>> I think we need another flag for settings that should be sent to replication
>> clients. GUC_REPORT_REPLICATION? Some settings would have both GUC_REPORT
>> and GUC_REPORT_REPLICATION, while others would have only one of them.
>
> Yup, seems like the cleanest solution.
Agreed. But when replication_timeout is changed by SIGHUP in the server,
there would be a delay before pg_receivexlog receives the parameter
change packet and changes the standby message interval based on the
new value of replication_timeout. Which might cause unexpected replication
timeout. So the user-settable interval (i.e., --statusint) is still useful for
people who want to avoid such fragility, even if we will support the auto-
tuning of the interval.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/doc/src/sgml/ref/pg_basebackup.sgml
--- b/doc/src/sgml/ref/pg_basebackup.sgml
***************
*** 335,341 **** PostgreSQL documentation
<term><option>--statusint=<replaceable class="parameter">interval</replaceable></option></term>
<listitem>
<para>
! Specifies the number of seconds between status packets sent back to the
server. This is required when streaming the transaction log (using
<literal>--xlog=stream</literal>) if replication timeout is configured
on the server, and allows for easier monitoring. The default value is
--- 335,341 ----
<term><option>--statusint=<replaceable class="parameter">interval</replaceable></option></term>
<listitem>
<para>
! Specifies the number of milliseconds between status packets sent back to the
server. This is required when streaming the transaction log (using
<literal>--xlog=stream</literal>) if replication timeout is configured
on the server, and allows for easier monitoring. The default value is
*** a/doc/src/sgml/ref/pg_receivexlog.sgml
--- b/doc/src/sgml/ref/pg_receivexlog.sgml
***************
*** 108,114 **** PostgreSQL documentation
<term><option>--statusint=<replaceable class="parameter">interval</replaceable></option></term>
<listitem>
<para>
! Specifies the number of seconds between status packets sent back to the
server. This is required if replication timeout is configured on the
server, and allows for easier monitoring. The default value is
10 seconds.
--- 108,114 ----
<term><option>--statusint=<replaceable class="parameter">interval</replaceable></option></term>
<listitem>
<para>
! Specifies the number of milliseconds between status packets sent back to the
server. This is required if replication timeout is configured on the
server, and allows for easier monitoring. The default value is
10 seconds.
*** a/src/bin/pg_basebackup/pg_basebackup.c
--- b/src/bin/pg_basebackup/pg_basebackup.c
***************
*** 46,52 **** int compresslevel = 0;
bool includewal = false;
bool streamwal = false;
bool fastcheckpoint = false;
! int standby_message_timeout = 10; /* 10 sec = default */
/* Progress counters */
static uint64 totalsize;
--- 46,52 ----
bool includewal = false;
bool streamwal = false;
bool fastcheckpoint = false;
! int standby_message_timeout = 10 * 1000; /* 10 sec = default */
/* Progress counters */
static uint64 totalsize;
***************
*** 118,124 **** usage(void)
printf(_(" --help show this help, then exit\n"));
printf(_(" --version output version information, then exit\n"));
printf(_("\nConnection options:\n"));
! printf(_(" -s, --statusint=INTERVAL time between status packets sent to server (in seconds)\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
printf(_(" -p, --port=PORT database server port number\n"));
printf(_(" -U, --username=NAME connect as specified database user\n"));
--- 118,124 ----
printf(_(" --help show this help, then exit\n"));
printf(_(" --version output version information, then exit\n"));
printf(_("\nConnection options:\n"));
! printf(_(" -s, --statusint=INTERVAL time between status packets sent to server (in milliseconds)\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
printf(_(" -p, --port=PORT database server port number\n"));
printf(_(" -U, --username=NAME connect as specified database user\n"));
*** a/src/bin/pg_basebackup/pg_receivexlog.c
--- b/src/bin/pg_basebackup/pg_receivexlog.c
***************
*** 36,42 ****
/* Global options */
char *basedir = NULL;
int verbose = 0;
! int standby_message_timeout = 10; /* 10 sec = default */
volatile bool time_to_abort = false;
--- 36,42 ----
/* Global options */
char *basedir = NULL;
int verbose = 0;
! int standby_message_timeout = 10 * 1000; /* 10 sec = default */
volatile bool time_to_abort = false;
***************
*** 59,65 **** usage(void)
printf(_(" -?, --help show this help, then exit\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_("\nConnection options:\n"));
! printf(_(" -s, --statusint=INTERVAL time between status packets sent to server (in seconds)\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
printf(_(" -p, --port=PORT database server port number\n"));
printf(_(" -U, --username=NAME connect as specified database user\n"));
--- 59,65 ----
printf(_(" -?, --help show this help, then exit\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_("\nConnection options:\n"));
! printf(_(" -s, --statusint=INTERVAL time between status packets sent to server (in milliseconds)\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
printf(_(" -p, --port=PORT database server port number\n"));
printf(_(" -U, --username=NAME connect as specified database user\n"));
*** a/src/bin/pg_basebackup/receivelog.c
--- b/src/bin/pg_basebackup/receivelog.c
***************
*** 62,68 **** open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir, char *namebu
f = open(fn, O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR);
if (f == -1)
{
! fprintf(stderr, _("%s: Could not open WAL segment %s: %s\n"),
progname, fn, strerror(errno));
return -1;
}
--- 62,68 ----
f = open(fn, O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR);
if (f == -1)
{
! fprintf(stderr, _("%s: could not open WAL segment %s: %s\n"),
progname, fn, strerror(errno));
return -1;
}
***************
*** 190,195 **** localGetCurrentTimestamp(void)
--- 190,250 ----
}
/*
+ * Local version of TimestampDifference(), since we are not
+ * linked with backend code.
+ */
+ static void
+ localTimestampDifference(TimestampTz start_time, TimestampTz stop_time,
+ long *secs, int *microsecs)
+ {
+ TimestampTz diff = stop_time - start_time;
+
+ if (diff <= 0)
+ {
+ *secs = 0;
+ *microsecs = 0;
+ }
+ else
+ {
+ #ifdef HAVE_INT64_TIMESTAMP
+ *secs = (long) (diff / USECS_PER_SEC);
+ *microsecs = (int) (diff % USECS_PER_SEC);
+ #else
+ *secs = (long) diff;
+ *microsecs = (int) ((diff - *secs) * 1000000.0);
+ #endif
+ }
+ }
+
+ /*
+ * Local version of TimestampDifferenceExceeds(), since we are not
+ * linked with backend code.
+ */
+ static bool
+ localTimestampDifferenceExceeds(TimestampTz start_time,
+ TimestampTz stop_time,
+ int msec)
+ {
+ TimestampTz diff = stop_time - start_time;
+
+ #ifdef HAVE_INT64_TIMESTAMP
+ return (diff >= msec * INT64CONST(1000));
+ #else
+ return (diff * 1000.0 >= msec);
+ #endif
+ }
+
+ /*
+ * Local version of TimestampTzPlusMilliseconds(), since we are not
+ * linked with backend code.
+ */
+ #ifdef HAVE_INT64_TIMESTAMP
+ #define localTimestampTzPlusMilliseconds(tz,ms) ((tz) + ((ms) * (int64) 1000))
+ #else
+ #define localTimestampTzPlusMilliseconds(tz,ms) ((tz) + ((ms) / 1000.0))
+ #endif
+
+ /*
* Receive a log stream starting at the specified position.
*
* If sysidentifier is specified, validate that both the system
***************
*** 207,213 **** localGetCurrentTimestamp(void)
* indefinitely.
*
* standby_message_timeout controls how often we send a message
! * back to the master letting it know our progress, in seconds.
* This message will only contain the write location, and never
* flush or replay.
*
--- 262,268 ----
* indefinitely.
*
* standby_message_timeout controls how often we send a message
! * back to the master letting it know our progress, in milliseconds.
* This message will only contain the write location, and never
* flush or replay.
*
***************
*** 301,307 **** ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline, char *sysi
*/
now = localGetCurrentTimestamp();
if (standby_message_timeout > 0 &&
! last_status < now - standby_message_timeout * 1000000)
{
/* Time to send feedback! */
char replybuf[sizeof(StandbyReplyMessage) + 1];
--- 356,363 ----
*/
now = localGetCurrentTimestamp();
if (standby_message_timeout > 0 &&
! localTimestampDifferenceExceeds(last_status, now,
! standby_message_timeout))
{
/* Time to send feedback! */
char replybuf[sizeof(StandbyReplyMessage) + 1];
***************
*** 340,349 **** ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline, char *sysi
FD_SET(PQsocket(conn), &input_mask);
if (standby_message_timeout)
{
! timeout.tv_sec = last_status + standby_message_timeout - now - 1;
! if (timeout.tv_sec <= 0)
! timeout.tv_sec = 1; /* Always sleep at least 1 sec */
! timeout.tv_usec = 0;
timeoutptr = &timeout;
}
else
--- 396,404 ----
FD_SET(PQsocket(conn), &input_mask);
if (standby_message_timeout)
{
! localTimestampDifference(now, localTimestampTzPlusMilliseconds(last_status,
! standby_message_timeout - 1),
! &timeout.tv_sec, (int *)&timeout.tv_usec);
timeoutptr = &timeout;
}
else
***************
*** 361,367 **** ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline, char *sysi
}
else if (r < 0)
{
! fprintf(stderr, _("%s: select() failed: %m\n"), progname);
return false;
}
/* Else there is actually data on the socket */
--- 416,422 ----
}
else if (r < 0)
{
! fprintf(stderr, _("%s: select() failed: %s\n"), progname, strerror(errno));
return false;
}
/* Else there is actually data on the socket */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
