On Tue, Feb 10, 2015 at 10:32 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Mon, Feb 9, 2015 at 8:29 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>> On Sun, Feb 8, 2015 at 2:54 PM, Michael Paquier
>> <michael.paqu...@gmail.com> wrote:
>>> On Fri, Feb 6, 2015 at 4:58 PM, Fujii Masao wrote:
>>>> - * Wait for more WAL to arrive. Time out after 5
>>>> seconds,
>>>> - * like when polling the archive, to react to a
>>>> trigger
>>>> - * file promptly.
>>>> + * Wait for more WAL to arrive. Time out after
>>>> the amount of
>>>> + * time specified by wal_retrieve_retry_interval, like
>>>> + * when polling the archive, to react to a
>>>> trigger file promptly.
>>>> */
>>>> WaitLatch(&XLogCtl->recoveryWakeupLatch,
>>>> WL_LATCH_SET | WL_TIMEOUT,
>>>> - 5000L);
>>>> + wal_retrieve_retry_interval * 1000L);
>>>>
>>>> This change can prevent the startup process from reacting to
>>>> a trigger file. Imagine the case where the large interval is set
>>>> and the user want to promote the standby by using the trigger file
>>>> instead of pg_ctl promote. I think that the sleep time should be 5s
>>>> if the interval is set to more than 5s. Thought?
>>>
>>> I disagree here. It is interesting to accelerate the check of WAL
>>> availability from a source in some cases for replication, but the
>>> opposite is true as well as mentioned by Alexey at the beginning of
>>> the thread to reduce the number of requests when requesting WAL
>>> archives from an external storage type AWS. Hence a correct solution
>>> would be to check periodically for the trigger file with a maximum
>>> one-time wait of 5s to ensure backward-compatible behavior. We could
>>> reduce it to 1s or something like that as well.
>>
>> You seem to have misunderstood the code in question. Or I'm missing
>> something.
>> The timeout of the WaitLatch is just the interval to check for the trigger
>> file
>> while waiting for more WAL to arrive from streaming replication. Not related
>> to
>> the retry time to restore WAL from the archive.
>
> [Re-reading the code...]
> Aah.. Yes you are right. Sorry for the noise. Yes let's wait for a
> maximum of 5s then. I also noticed in previous patch that the wait was
> maximized to 5s. To begin with, a loop should have been used if it was
> a sleep, but as now patch uses a latch this limit does not make much
> sense... Patch updated is attached.
On second thought, the interval to check the trigger file is very different
from the wait time to retry to retrieve WAL. So it seems strange and even
confusing to control them by one parameter. If we really want to make
the interval for the trigger file configurable, we should invent new GUC for it.
But I don't think that it's worth doing that. If someone wants to react the
trigger file more promptly for "fast" promotion, he or she basically can use
pg_ctl promote command, instead. Thought?
Attached is the updated version of the patch. I changed the parameter so that
it doesn't affect the interval of checking the trigger file.
- static pg_time_t last_fail_time = 0;
- pg_time_t now;
+ TimestampTz now = GetCurrentTimestamp();
+ TimestampTz last_fail_time = now;
I reverted the code here as it was. I don't think GetCurrentTimestamp() needs
to be called for each WaitForWALToBecomeAvailable().
+ WaitLatch(&XLogCtl->recoveryWakeupLatch,
+ WL_LATCH_SET | WL_TIMEOUT,
+ wait_time / 1000);
Don't we need to specify WL_POSTMASTER_DEATH flag here? Added.
+ {"wal_retrieve_retry_interval", PGC_SIGHUP, WAL_SETTINGS,
WAL_SETTINGS should be REPLICATION_STANDBY? Changed.
Regards,
--
Fujii Masao
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 2985,2990 **** include_dir 'conf.d'
--- 2985,3008 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-wal-retrieve-retry-interval" xreflabel="wal_retrieve_retry_interval">
+ <term><varname>wal_retrieve_retry_interval</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>wal_retrieve_retry_interval</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specify the amount of time, in milliseconds, to wait when WAL is not
+ available from any sources (streaming replication,
+ local <filename>pg_xlog</> or WAL archive) before retrying to
+ retrieve WAL. This parameter can only be set in the
+ <filename>postgresql.conf</> file or on the server command line.
+ The default value is 5 seconds.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
</sect1>
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 93,98 **** int sync_method = DEFAULT_SYNC_METHOD;
--- 93,99 ----
int wal_level = WAL_LEVEL_MINIMAL;
int CommitDelay = 0; /* precommit delay in microseconds */
int CommitSiblings = 5; /* # concurrent xacts needed to sleep */
+ int wal_retrieve_retry_interval = 5000;
#ifdef WAL_DEBUG
bool XLOG_DEBUG = false;
***************
*** 10340,10347 **** static bool
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
bool fetching_ckpt, XLogRecPtr tliRecPtr)
{
! static pg_time_t last_fail_time = 0;
! pg_time_t now;
/*-------
* Standby mode is implemented by a state machine:
--- 10341,10348 ----
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
bool fetching_ckpt, XLogRecPtr tliRecPtr)
{
! static TimestampTz last_fail_time = 0;
! TimestampTz now;
/*-------
* Standby mode is implemented by a state machine:
***************
*** 10351,10357 **** WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
* 2. Check trigger file
* 3. Read from primary server via walreceiver (XLOG_FROM_STREAM)
* 4. Rescan timelines
! * 5. Sleep 5 seconds, and loop back to 1.
*
* Failure to read from the current source advances the state machine to
* the next state.
--- 10352,10358 ----
* 2. Check trigger file
* 3. Read from primary server via walreceiver (XLOG_FROM_STREAM)
* 4. Rescan timelines
! * 5. Sleep wal_retrieve_retry_interval milliseconds, and loop back to 1.
*
* Failure to read from the current source advances the state machine to
* the next state.
***************
*** 10490,10503 **** WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
* machine, so we've exhausted all the options for
* obtaining the requested WAL. We're going to loop back
* and retry from the archive, but if it hasn't been long
! * since last attempt, sleep 5 seconds to avoid
! * busy-waiting.
*/
! now = (pg_time_t) time(NULL);
! if ((now - last_fail_time) < 5)
{
! pg_usleep(1000000L * (5 - (now - last_fail_time)));
! now = (pg_time_t) time(NULL);
}
last_fail_time = now;
currentSource = XLOG_FROM_ARCHIVE;
--- 10491,10515 ----
* machine, so we've exhausted all the options for
* obtaining the requested WAL. We're going to loop back
* and retry from the archive, but if it hasn't been long
! * since last attempt, sleep wal_retrieve_retry_interval
! * milliseconds to avoid busy-waiting.
*/
! now = GetCurrentTimestamp();
! if (!TimestampDifferenceExceeds(last_fail_time, now,
! wal_retrieve_retry_interval))
{
! long secs, wait_time;
! int usecs;
!
! TimestampDifference(last_fail_time, now, &secs, &usecs);
! wait_time = wal_retrieve_retry_interval -
! (secs * 1000 + usecs / 1000);
!
! WaitLatch(&XLogCtl->recoveryWakeupLatch,
! WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! wait_time);
! ResetLatch(&XLogCtl->recoveryWakeupLatch);
! now = GetCurrentTimestamp();
}
last_fail_time = now;
currentSource = XLOG_FROM_ARCHIVE;
***************
*** 10653,10664 **** WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
}
/*
! * Wait for more WAL to arrive. Time out after 5 seconds,
! * like when polling the archive, to react to a trigger
! * file promptly.
*/
WaitLatch(&XLogCtl->recoveryWakeupLatch,
! WL_LATCH_SET | WL_TIMEOUT,
5000L);
ResetLatch(&XLogCtl->recoveryWakeupLatch);
break;
--- 10665,10675 ----
}
/*
! * Wait for more WAL to arrive. Time out after 5 seconds
! * to react to a trigger file promptly.
*/
WaitLatch(&XLogCtl->recoveryWakeupLatch,
! WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
5000L);
ResetLatch(&XLogCtl->recoveryWakeupLatch);
break;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 2364,2369 **** static struct config_int ConfigureNamesInt[] =
--- 2364,2381 ----
},
{
+ {"wal_retrieve_retry_interval", PGC_SIGHUP, REPLICATION_STANDBY,
+ gettext_noop("Sets the time to wait before retrying to retrieve WAL"
+ "after a failed attempt."),
+ NULL,
+ GUC_UNIT_MS
+ },
+ &wal_retrieve_retry_interval,
+ 5000, 1, INT_MAX,
+ NULL, NULL, NULL
+ },
+
+ {
{"wal_segment_size", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the number of pages per write ahead log segment."),
NULL,
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 260,265 ****
--- 260,267 ----
#wal_receiver_timeout = 60s # time that receiver waits for
# communication from master
# in milliseconds; 0 disables
+ #wal_retrieve_retry_interval = 5s # time to wait before retrying to
+ # retrieve WAL after a failed attempt
#------------------------------------------------------------------------------
*** a/src/include/access/xlog.h
--- b/src/include/access/xlog.h
***************
*** 93,98 **** extern int CheckPointSegments;
--- 93,99 ----
extern int wal_keep_segments;
extern int XLOGbuffers;
extern int XLogArchiveTimeout;
+ extern int wal_retrieve_retry_interval;
extern bool XLogArchiveMode;
extern char *XLogArchiveCommand;
extern bool EnableHotStandby;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
