Navigation Menu

Skip to content

Commit

Permalink
Fixed an issue where asynchronous archiving was transferring one file…
Browse files Browse the repository at this point in the history
… per execution instead of transferring files in batches.

This regression was introduced in v1.09 and affected efficiency only, all WAL segments were correctly archived in asynchronous mode.

Reported by Stephen Frost.
  • Loading branch information
dwsteele committed Nov 17, 2016
1 parent 06cac30 commit dbb9d80
Show file tree
Hide file tree
Showing 16 changed files with 699 additions and 742 deletions.
14 changes: 14 additions & 0 deletions doc/xml/release.xml
Expand Up @@ -133,6 +133,20 @@
</contributor-list>

<release-list>
<release date="XXXX-XX-XX" version="1.11dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-bug-list>
<release-item>
<release-item-contributor-list>
<release-item-ideator id="frost.stephen"/>
</release-item-contributor-list>

<p>Fixed an issue where asynchronous archiving was transferring one file per execution instead of transferring files in batches. This regression was introduced in <id>v1.09</id> and affected efficiency only, all WAL segments were correctly archived in asynchronous mode.</p>
</release-item>
</release-bug-list>
</release-core-list>
</release>

<release date="2016-11-08" version="1.10" title="Stanza Creation and Minor Bug Fixes">
<release-core-list>
<release-bug-list>
Expand Down
34 changes: 34 additions & 0 deletions doc/xml/user-guide.xml
Expand Up @@ -1466,6 +1466,40 @@
<exe-highlight>WAL segment</exe-highlight>
</execute>
</execute-list>

<!-- DEBUG test to ensure async archiving is working in batch until better tests can be written -->
<execute-list host="{[host-db-master]}" keyword="debug">
<title>DEBUG asynchronous archiving</title>

<execute output="n">
<exe-cmd>rm /var/log/pgbackrest/demo-archive-async.log</exe-cmd>
</execute>

<execute output="n">
<exe-cmd>
psql -c "
select pg_create_restore_point('test asynchronous archiving');
select pg_switch_xlog();
select pg_create_restore_point('test asynchronous archiving');
select pg_switch_xlog();
select pg_create_restore_point('test asynchronous archiving');
select pg_switch_xlog();
select pg_create_restore_point('test asynchronous archiving');
select pg_switch_xlog();
select pg_create_restore_point('test asynchronous archiving');
select pg_switch_xlog();"
</exe-cmd>
</execute>

<execute output="n">
<exe-cmd>sleep 5</exe-cmd>
</execute>

<execute output="y">
<exe-cmd>cat /var/log/pgbackrest/demo-archive-async.log</exe-cmd>
<exe-highlight>WAL segments to archive</exe-highlight>
</execute>
</execute-list>
</section>
</section>

Expand Down
86 changes: 49 additions & 37 deletions lib/pgBackRest/Archive.pm
Expand Up @@ -573,8 +573,8 @@ sub pushProcess
confess &log(ERROR, CMD_ARCHIVE_PUSH . ' operation must run on the db host');
}

# Load the archive object
use pgBackRest::Archive;
# Batch flag indicates that the archive-push command was called without a WAL segment
my $bBatch = false;

# If an archive section has been defined, use that instead of the backup section when command is CMD_ARCHIVE_PUSH
my $bArchiveAsync = optionGet(OPTION_ARCHIVE_ASYNC);
Expand Down Expand Up @@ -617,64 +617,76 @@ sub pushProcess
{
$oException = $EVAL_ERROR;

if (!$bArchiveAsync || !optionTest(OPTION_TEST_NO_FORK))
if (!$bArchiveAsync || optionGet(OPTION_TEST_NO_FORK))
{
confess $oException;
}
};

# Fork if async archiving is enabled
if ($bArchiveAsync)
{
# Fork and disable the async archive flag if this is the parent process
if (!optionTest(OPTION_TEST_NO_FORK))
{
$bArchiveAsync = fork() == 0 ? true : false;
}
# Else the no-fork flag has been specified for testing
else
{
logDebugMisc($strOperation, 'no fork on archive local for TESTING');
}
}
}

if ($bArchiveAsync)
# If archive-push is called without a WAL segment then still run in batch mode to clear out the async queue
else
{
# Start the async archive push
logDebugMisc($strOperation, 'start async archive-push');
$bBatch = true;
}

# If async or batch mode then acquire a lock. Also fork if in async mode so that the parent process can return to Postres.
if ($bArchiveAsync || $bBatch)
{
# Create a lock file to make sure async archive-push does not run more than once
if (!lockAcquire(commandGet(), false))
{
logDebugMisc($strOperation, 'async archive-push process is already running - exiting');
$bBatch = false;
}
else
{
# Open the log file
logFileSet(optionGet(OPTION_LOG_PATH) . '/' . optionGet(OPTION_STANZA) . '-archive-async');

# Call the archive_xfer function and continue to loop as long as there are files to process
my $iLogTotal;

while (!defined($iLogTotal) || $iLogTotal > 0)
# Only fork if a WAL segment was specified, otherwise jut run
if (!$bBatch)
{
$iLogTotal = $self->xfer(optionGet(OPTION_SPOOL_PATH) . "/archive/" .
optionGet(OPTION_STANZA) . "/out", $strStopFile);

if ($iLogTotal > 0)
# Fork and disable the async archive flag if this is the parent process
if (!optionGet(OPTION_TEST_NO_FORK))
{
logDebugMisc($strOperation, "transferred ${iLogTotal} WAL segment" .
($iLogTotal > 1 ? 's' : '') . ', calling Archive->xfer() again');
$bBatch = fork() == 0 ? true : false;
}
# Else the no-fork flag has been specified for testing
else
{
logDebugMisc($strOperation, 'transfer found 0 WAL segments - exiting');
logDebugMisc($strOperation, 'no fork on archive local for TESTING');
$bBatch = true;
}
}
}
}

# Continue with batch processing
if ($bBatch)
{
# Start the async archive push
logDebugMisc($strOperation, 'start async archive-push');

# Open the log file
logFileSet(optionGet(OPTION_LOG_PATH) . '/' . optionGet(OPTION_STANZA) . '-archive-async');

# Call the archive_xfer function and continue to loop as long as there are files to process
my $iLogTotal;

lockRelease();
while (!defined($iLogTotal) || $iLogTotal > 0)
{
$iLogTotal = $self->xfer(optionGet(OPTION_SPOOL_PATH) . "/archive/" .
optionGet(OPTION_STANZA) . "/out", $strStopFile);

if ($iLogTotal > 0)
{
logDebugMisc($strOperation, "transferred ${iLogTotal} WAL segment" .
($iLogTotal > 1 ? 's' : '') . ', calling Archive->xfer() again');
}
else
{
logDebugMisc($strOperation, 'transfer found 0 WAL segments - exiting');
}
}

lockRelease();
}
elsif (defined($oException))
{
Expand Down
2 changes: 1 addition & 1 deletion lib/pgBackRest/Version.pm
Expand Up @@ -35,7 +35,7 @@ use constant BACKREST_BIN => abs_path(
# Defines the current version of the BackRest executable. The version number is used to track features but does not affect what
# repositories or manifests can be read - that's the job of the format number.
#-----------------------------------------------------------------------------------------------------------------------------------
use constant BACKREST_VERSION => '1.10';
use constant BACKREST_VERSION => '1.11dev';
push @EXPORT, qw(BACKREST_VERSION);

# Format Format Number
Expand Down

0 comments on commit dbb9d80

Please sign in to comment.