db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-03-03 14:52:21 +02:00
Code Issues Releases Activity

Suppress errors when closing local/remote processes.

Browse Source 
Since the command has completed it is counterproductive to throw an error but still warn to indicate that something unusual happened.

Also fix the related issue that the local processes were not being shut down when they completed, which meant that they might timeout before being closed when pgbackrest terminated.
This commit is contained in:
David Steele 2020-07-28 12:15:33 -04:00 committed by GitHub
parent d9309f13a4
commit 63a93db6fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 123 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/xml/release.xml
View File

@ -13,6 +13,21 @@
<release-list>
<release date="XXXX-XX-XX" version="2.29dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-bug-list>
<release-item>
<release-item-contributor-list>
<release-item-ideator id="argdenis"/>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>
<p>Suppress errors when closing <cmd>local</cmd>/<cmd>remote</cmd> processes.</p>
<p>Since the command has completed it is counterproductive to throw an error but still <b>warn</b> to indicate that something unusual happened.</p>
</release-item>
</release-bug-list>
</release-core-list>
<release-doc-list>
<release-improvement-list>
<release-item>
@ -8376,6 +8391,11 @@
<contributor-id type="github">trinchan</contributor-id>
</contributor>
<contributor id="argdenis">
<contributor-name-display>argdenis</contributor-name-display>
<contributor-id type="github">argdenis</contributor-id>
</contributor>
<contributor id="benoit.lobréau">
<contributor-name-display>blogh</contributor-name-display>
<contributor-id type="github">blogh</contributor-id>

8
src/common/exit.c
View File

@ -120,12 +120,8 @@ exitSafe(int result, bool error, SignalType signalType)
result = errorCode();
}
// Free protocol objects but ignore errors
TRY_BEGIN()
{
protocolFree();
}
TRY_END();
// Free protocol objects
protocolFree();
// Log command end if a command is set
if (cfgCommand() != cfgCmdNone)

88
src/protocol/helper.c
View File

@ -217,6 +217,67 @@ protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int hostId, u
FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient->client);
}
/***********************************************************************************************************************************
Free the protocol client and underlying exec'd process. Log any errors as warnings since it is not worth terminating the process
while closing a local/remote that has already completed its work. The warning will be an indication that something is not right.
***********************************************************************************************************************************/
static void
protocolHelperClientFree(ProtocolHelperClient *protocolHelperClient)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM_P(VOID, protocolHelperClient);
FUNCTION_LOG_END();
if (protocolHelperClient->client != NULL)
{
// Try to shutdown the protocol but only warn on error
TRY_BEGIN()
{
protocolClientFree(protocolHelperClient->client);
}
CATCH_ANY()
{
LOG_WARN(errorMessage());
}
TRY_END();
// Try to end the child process but only warn on error
TRY_BEGIN()
{
execFree(protocolHelperClient->exec);
}
CATCH_ANY()
{
LOG_WARN(errorMessage());
}
TRY_END();
protocolHelperClient->client = NULL;
protocolHelperClient->exec = NULL;
}
FUNCTION_LOG_RETURN_VOID();
}
/**********************************************************************************************************************************/
void
protocolLocalFree(unsigned int protocolId)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(UINT, protocolId);
FUNCTION_LOG_END();
ASSERT(protocolId > 0);
if (protocolHelper.clientLocal != NULL)
{
ASSERT(protocolId <= protocolHelper.clientLocalSize);
protocolHelperClientFree(&protocolHelper.clientLocal[protocolId - 1]);
}
FUNCTION_LOG_RETURN_VOID();
}
/***********************************************************************************************************************************
Get the command line required for remote protocol execution
***********************************************************************************************************************************/
@ -493,18 +554,7 @@ protocolRemoteFree(unsigned int hostId)
ASSERT(hostId > 0);
if (protocolHelper.clientRemote != NULL)
{
ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientRemote[hostId - 1];
if (protocolHelperClient->client != NULL)
{
protocolClientFree(protocolHelperClient->client);
execFree(protocolHelperClient->exec);
protocolHelperClient->client = NULL;
protocolHelperClient->exec = NULL;
}
}
protocolHelperClientFree(&protocolHelper.clientRemote[hostId - 1]);
FUNCTION_LOG_RETURN_VOID();
}
@ -579,18 +629,8 @@ protocolFree(void)
protocolRemoteFree(clientIdx + 1);
// Free locals
for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientLocalSize; clientIdx++)
{
ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientLocal[clientIdx];
if (protocolHelperClient->client != NULL)
{
protocolClientFree(protocolHelperClient->client);
execFree(protocolHelperClient->exec);
*protocolHelperClient = (ProtocolHelperClient){.exec = NULL};
}
}
for (unsigned int clientIdx = 1; clientIdx <= protocolHelper.clientLocalSize; clientIdx++)
protocolLocalFree(clientIdx);
}
FUNCTION_LOG_RETURN_VOID();

3
src/protocol/helper.h
View File

@ -35,6 +35,9 @@ void protocolKeepAlive(void);
// Local protocol client
ProtocolClient *protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int hostId, unsigned int protocolId);
// Free (shutdown) a local
void protocolLocalFree(unsigned int protocolId);
// Remote protocol client
ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostId);

4
src/protocol/parallel.c
View File

@ -14,6 +14,7 @@ Protocol Parallel Executor
#include "common/type/list.h"
#include "common/type/object.h"
#include "protocol/command.h"
#include "protocol/helper.h"
#include "protocol/parallel.h"
/***********************************************************************************************************************************
@ -217,6 +218,9 @@ protocolParallelProcess(ProtocolParallel *this)
protocolParallelJobStateSet(job, protocolParallelJobStateRunning);
this->clientJobList[clientIdx] = job;
}
// Else no more jobs for this client so free it
else
protocolLocalFree(clientIdx + 1);
}
}

5
test/define.yaml
View File

@ -477,7 +477,7 @@ unit:
test:
# ----------------------------------------------------------------------------------------------------------------------------
- name: protocol
total: 9
total: 10
containerReq: true
binReq: true
@ -489,6 +489,9 @@ unit:
protocol/parallelJob: full
protocol/server: full
include:
- common/exec
# ********************************************************************************************************************************
- name: info

26
test/src/module/protocol/protocolTest.c
View File

@ -195,6 +195,27 @@ testRun(void)
TEST_RESULT_BOOL(pgIsLocal(7), false, "pg is remote");
}
// *****************************************************************************************************************************
if (testBegin("protocolHelperClientFree()"))
{
TEST_TITLE("free with errors output as warnings");
// Create and free a mem context to give us an error to use
MemContext *memContext = memContextNew("test");
memContextFree(memContext);
// Create bogus client and exec with the freed memcontext to generate errors
ProtocolClient client = {.memContext = memContext, .name = STRDEF("test")};
Exec exec = {.memContext = memContext, .name = STRDEF("test"), .command = strNew("test")};
ProtocolHelperClient protocolHelperClient = {.client = &client, .exec = &exec};
TEST_RESULT_VOID(protocolHelperClientFree(&protocolHelperClient), "free");
harnessLogResult(
"P00 WARN: cannot free inactive context\n"
"P00 WARN: cannot free inactive context");
}
// *****************************************************************************************************************************
if (testBegin("protocolLocalParam()"))
{
@ -890,6 +911,11 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(protocolRemoteFree(1), "free remote (non exist)");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("free local that does not exist");
TEST_RESULT_VOID(protocolLocalFree(2), "free");
// Call keep alive before any remotes exist
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(protocolKeepAlive(), "keep alive");