db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-14 10:13:05 +02:00
Code Issues Releases Activity

Move noop from protocolClientNew() to local/remote initialization.

Browse Source 
protocolServerNew() does not automatically process a noop so this made the handshake in the constructors asymmetric. This made testing a bit confusing since an extra noop was needed when cmdLocal()/cmdRemote() were not called (since they processed the noop sent by protocolClientNew()).

The extra noops also make complex protocol negotiation (coming in a future commit) more complicated and slower because of the additional round trips.
This commit is contained in:
David Steele 2021-07-23 08:32:30 -04:00
parent 4c9ddf5ef2
commit 0999de0279
4 changed files with 47 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/protocol/client.c
View File

@ -128,9 +128,6 @@ protocolClientNew(const String *name, const String *service, IoRead *read, IoWri
}
MEM_CONTEXT_TEMP_END();
// Send one noop to catch any errors that might happen after the greeting
protocolClientNoOp(this);
// Set a callback to shutdown the protocol
memContextCallbackSet(this->pub.memContext, protocolClientFreeResource, this);
}

8
src/protocol/helper.c
View File

@ -196,6 +196,7 @@ protocolLocalExec(
strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u protocol", processId),
PROTOCOL_SERVICE_LOCAL_STR, execIoRead(helper->exec), execIoWrite(helper->exec));
// Move client to exec context so they are freed together
protocolClientMove(helper->client, execMemContext(helper->exec));
FUNCTION_TEST_RETURN_VOID();
@ -238,6 +239,9 @@ protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int hostIdx,
protocolLocalExec(protocolHelperClient, protocolStorageType, hostIdx, processId);
}
MEM_CONTEXT_END();
// Send noop to catch initialization errors
protocolClientNoOp(protocolHelperClient->client);
}
FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient->client);
@ -507,6 +511,7 @@ protocolRemoteExec(
strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u protocol on '%s'", processId, host), PROTOCOL_SERVICE_REMOTE_STR,
execIoRead(helper->exec), execIoWrite(helper->exec));
// Move client to exec context so they are freed together
protocolClientMove(helper->client, execMemContext(helper->exec));
FUNCTION_TEST_RETURN_VOID();
@ -558,6 +563,9 @@ protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostIdx)
{
protocolRemoteExec(protocolHelperClient, protocolStorageType, hostIdx, processId);
// Send noop to catch initialization errors
protocolClientNoOp(protocolHelperClient->client);
// Get cipher options from the remote if none are locally configured
if (isRepo && cfgOptionIdxStrId(cfgOptRepoCipherType, hostIdx) == cipherTypeNone)
{

40
test/src/module/command/remoteTest.c
View File

@ -110,12 +110,27 @@ testRun(void)
HRN_FORK_PARENT_BEGIN()
{
TEST_ERROR(
ProtocolClient *client = NULL;
TEST_ASSIGN(
client,
protocolClientNew(
STRDEF("test"), PROTOCOL_SERVICE_REMOTE_STR,
ioFdReadNewOpen(STRDEF("server read"), HRN_FORK_PARENT_READ_FD(0), 2000),
ioFdWriteNewOpen(STRDEF("server write"), HRN_FORK_PARENT_WRITE_FD(0), 2000)),
PathCreateError, "raised from test: unable to create path '/bogus': [13] Permission denied");
ioFdWriteNewOpen(STRDEF("server write"), HRN_FORK_PARENT_WRITE_FD(0), 2000)), "new client");
TEST_ERROR(
protocolClientNoOp(client), PathCreateError,
"raised from test: unable to create path '/bogus': [13] Permission denied");
// The server is in an error state so ignore any client errors
TRY_BEGIN()
{
protocolClientFree(client);
}
CATCH_ANY()
{
}
TRY_END();
}
HRN_FORK_PARENT_END();
}
@ -180,12 +195,25 @@ testRun(void)
{
HRN_STORAGE_PUT_EMPTY(hrnStorage, "lock/all" STOP_FILE_EXT);
TEST_ERROR(
ProtocolClient *client = NULL;
TEST_ASSIGN(
client,
protocolClientNew(
STRDEF("test"), PROTOCOL_SERVICE_REMOTE_STR,
ioFdReadNewOpen(STRDEF("server read"), HRN_FORK_PARENT_READ_FD(0), 2000),
ioFdWriteNewOpen(STRDEF("server write"), HRN_FORK_PARENT_WRITE_FD(0), 2000)),
StopError, "raised from test: stop file exists for all stanzas");
ioFdWriteNewOpen(STRDEF("server write"), HRN_FORK_PARENT_WRITE_FD(0), 2000)), "new client");
TEST_ERROR(protocolClientNoOp(client), StopError, "raised from test: stop file exists for all stanzas");
// The server is in an error state so ignore any client errors
TRY_BEGIN()
{
protocolClientFree(client);
}
CATCH_ANY()
{
}
TRY_END();
HRN_STORAGE_REMOVE(hrnStorage, "lock/all" STOP_FILE_EXT);
}

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

@ -572,6 +572,11 @@ testRun(void)
harnessLogLevelReset();
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("noop command");
TEST_RESULT_VOID(protocolClientNoOp(client), "noop");
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("simple command");
@ -672,9 +677,6 @@ testRun(void)
protocolServerNew(STRDEF("local server 1"), STRDEF("test"), HRN_FORK_CHILD_READ(), HRN_FORK_CHILD_WRITE()),
"local server 1");
TEST_RESULT_UINT(protocolServerCommandGet(server).id, PROTOCOL_COMMAND_NOOP, "noop command get");
TEST_RESULT_VOID(protocolServerDataEndPut(server), "data end put");
// Command with output
TEST_RESULT_UINT(protocolServerCommandGet(server).id, strIdFromZ(stringIdBit5, "c-one"), "c-one command get");
@ -698,9 +700,6 @@ testRun(void)
protocolServerNew(STRDEF("local server 2"), STRDEF("test"), HRN_FORK_CHILD_READ(), HRN_FORK_CHILD_WRITE()),
"local server 2");
TEST_RESULT_UINT(protocolServerCommandGet(server).id, PROTOCOL_COMMAND_NOOP, "noop command get");
TEST_RESULT_VOID(protocolServerDataEndPut(server), "data end put");
// Command with output
TEST_RESULT_UINT(protocolServerCommandGet(server).id, strIdFromZ(stringIdBit5, "c2"), "c2 command get");