Skip to content

Commit

Permalink
Add StringId type.
Browse files Browse the repository at this point in the history
It is often useful to represent identifiers as strings when they cannot easily be represented as an enum/integer, e.g. because they are distributed among a number of unrelated modules or need to be passed to remote processes. Strings are also more helpful in debugging since they can be recognized without cross-referencing the source. However, strings are awkward to work with in C since they cannot be directly used in switch statements leading to less efficient if-else structures.

A StringId encodes a short string into an integer so it can be used in switch statements but may also be readily converted back into a string for debugging purposes. StringIds may also be suitable for matching user input providing the strings are short enough.

This patch includes a sample of StringId usage by converting protocol commands to StringIds. There are many other possible use cases. To list a few:

* All "types" in storage, filters. IO , etc. These types are primarily for identification and debugging so they fit well with this model.

* MemContext names would work well as StringIds since these are entirely for debugging.

* Option values could be represented as StringIds which would mean we could remove the functions that convert strings to enums, e.g. CipherType.

* There are a number of places where enums need to be converted back to strings for logging/debugging purposes. An example is protocolParallelJobToConstZ. If ProtocolParallelJobState were defined as:

typedef enum
{
    protocolParallelJobStatePending = STRID5("pend", ...),
    protocolParallelJobStateRunning = STRID5("run", ...),
    protocolParallelJobStateDone = STRID5("done", ...),
} ProtocolParallelJobState;

then protocolParallelJobToConstZ() could be replaced with strIdToZ(). This also applies to many enums that we don't covert to strings for logging, such as CipherMode.

As an example of usage, convert all protocol commands from strings to StringIds.
  • Loading branch information
dwsteele committed Apr 20, 2021
1 parent 292f836 commit ed0d48f
Show file tree
Hide file tree
Showing 39 changed files with 787 additions and 194 deletions.
12 changes: 12 additions & 0 deletions doc/xml/release.xml
Expand Up @@ -27,6 +27,18 @@
<p>Add <br-option>db-exclude</br-option> option.</p>
</release-item>
</release-improvement-list>

<release-development-list>
<release-item>
<github-pull-request id="1358"/>

<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>

<p>Add StringId type.</p>
</release-item>
</release-development-list>
</release-core-list>

<release-doc-list>
Expand Down
1 change: 1 addition & 0 deletions src/Makefile.in
Expand Up @@ -111,6 +111,7 @@ SRCS = \
common/type/object.c \
common/type/pack.c \
common/type/string.c \
common/type/stringId.c \
common/type/stringList.c \
common/type/variant.c \
common/type/variantList.c \
Expand Down
2 changes: 1 addition & 1 deletion src/command/archive/get/get.c
Expand Up @@ -828,7 +828,7 @@ static ProtocolParallelJob *archiveGetAsyncCallback(void *data, unsigned int cli
const ArchiveFileMap *archiveFileMap = lstGet(jobData->archiveFileMapList, jobData->archiveFileIdx);
jobData->archiveFileIdx++;

ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_ARCHIVE_GET_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_ARCHIVE_GET_FILE);
protocolCommandParamAdd(command, VARSTR(archiveFileMap->request));

// Add actual files to get
Expand Down
5 changes: 0 additions & 5 deletions src/command/archive/get/protocol.c
Expand Up @@ -13,11 +13,6 @@ Archive Get Protocol Handler
#include "storage/helper.h"
#include "storage/write.intern.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
STRING_EXTERN(PROTOCOL_COMMAND_ARCHIVE_GET_FILE_STR, PROTOCOL_COMMAND_ARCHIVE_GET_FILE);

/**********************************************************************************************************************************/
void
archiveGetFileProtocol(const VariantList *paramList, ProtocolServer *server)
Expand Down
9 changes: 3 additions & 6 deletions src/command/archive/get/protocol.h
Expand Up @@ -5,15 +5,10 @@ Archive Get Protocol Handler
#define COMMAND_ARCHIVE_GET_PROTOCOL_H

#include "common/type/string.h"
#include "common/type/stringId.h"
#include "common/type/variantList.h"
#include "protocol/server.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_ARCHIVE_GET_FILE "archiveGetFile"
STRING_DECLARE(PROTOCOL_COMMAND_ARCHIVE_GET_FILE_STR);

/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
Expand All @@ -23,6 +18,8 @@ void archiveGetFileProtocol(const VariantList *paramList, ProtocolServer *server
/***********************************************************************************************************************************
Protocol commands for ProtocolServerHandler arrays passed to protocolServerProcess()
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_ARCHIVE_GET_FILE STRID5("ag-f", 0x36ce10)

#define PROTOCOL_SERVER_HANDLER_ARCHIVE_GET_LIST \
{.command = PROTOCOL_COMMAND_ARCHIVE_GET_FILE, .handler = archiveGetFileProtocol},

Expand Down
5 changes: 0 additions & 5 deletions src/command/archive/push/protocol.c
Expand Up @@ -12,11 +12,6 @@ Archive Push Protocol Handler
#include "config/config.h"
#include "storage/helper.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
STRING_EXTERN(PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE_STR, PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE);

/**********************************************************************************************************************************/
void
archivePushFileProtocol(const VariantList *paramList, ProtocolServer *server)
Expand Down
8 changes: 2 additions & 6 deletions src/command/archive/push/protocol.h
Expand Up @@ -8,12 +8,6 @@ Archive Push Protocol Handler
#include "common/type/variantList.h"
#include "protocol/server.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE "archivePushFile"
STRING_DECLARE(PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE_STR);

/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
Expand All @@ -23,6 +17,8 @@ void archivePushFileProtocol(const VariantList *paramList, ProtocolServer *serve
/***********************************************************************************************************************************
Protocol commands for ProtocolServerHandler arrays passed to protocolServerProcess()
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE STRID5("ap-f", 0x36e010)

#define PROTOCOL_SERVER_HANDLER_ARCHIVE_PUSH_LIST \
{.command = PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE, .handler = archivePushFileProtocol},

Expand Down
2 changes: 1 addition & 1 deletion src/command/archive/push/push.c
Expand Up @@ -460,7 +460,7 @@ archivePushAsyncCallback(void *data, unsigned int clientIdx)
const String *walFile = strLstGet(jobData->walFileList, jobData->walFileIdx);
jobData->walFileIdx++;

ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_ARCHIVE_PUSH_FILE);
protocolCommandParamAdd(command, VARSTR(strNewFmt("%s/%s", strZ(jobData->walPath), strZ(walFile))));
protocolCommandParamAdd(command, VARBOOL(cfgOptionBool(cfgOptArchiveHeaderCheck)));
protocolCommandParamAdd(command, VARUINT(jobData->archiveInfo.pgVersion));
Expand Down
2 changes: 1 addition & 1 deletion src/command/backup/backup.c
Expand Up @@ -1478,7 +1478,7 @@ static ProtocolParallelJob *backupJobCallback(void *data, unsigned int clientIdx
const ManifestFile *file = *(ManifestFile **)lstGet(queue, 0);

// Create backup job
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_BACKUP_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_BACKUP_FILE);

protocolCommandParamAdd(command, VARSTR(manifestPathPg(file->name)));
protocolCommandParamAdd(
Expand Down
5 changes: 0 additions & 5 deletions src/command/backup/protocol.c
Expand Up @@ -12,11 +12,6 @@ Backup Protocol Handler
#include "config/config.h"
#include "storage/helper.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
STRING_EXTERN(PROTOCOL_COMMAND_BACKUP_FILE_STR, PROTOCOL_COMMAND_BACKUP_FILE);

/**********************************************************************************************************************************/
void
backupFileProtocol(const VariantList *paramList, ProtocolServer *server)
Expand Down
8 changes: 2 additions & 6 deletions src/command/backup/protocol.h
Expand Up @@ -8,12 +8,6 @@ Backup Protocol Handler
#include "common/type/variantList.h"
#include "protocol/server.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_BACKUP_FILE "backupFile"
STRING_DECLARE(PROTOCOL_COMMAND_BACKUP_FILE_STR);

/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
Expand All @@ -23,6 +17,8 @@ void backupFileProtocol(const VariantList *paramList, ProtocolServer *server);
/***********************************************************************************************************************************
Protocol commands for ProtocolServerHandler arrays passed to protocolServerProcess()
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_BACKUP_FILE STRID5("bp-f", 0x36e020)

#define PROTOCOL_SERVER_HANDLER_BACKUP_LIST \
{.command = PROTOCOL_COMMAND_BACKUP_FILE, .handler = backupFileProtocol},

Expand Down
5 changes: 0 additions & 5 deletions src/command/restore/protocol.c
Expand Up @@ -12,11 +12,6 @@ Restore Protocol Handler
#include "config/config.h"
#include "storage/helper.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
STRING_EXTERN(PROTOCOL_COMMAND_RESTORE_FILE_STR, PROTOCOL_COMMAND_RESTORE_FILE);

/**********************************************************************************************************************************/
void
restoreFileProtocol(const VariantList *paramList, ProtocolServer *server)
Expand Down
8 changes: 2 additions & 6 deletions src/command/restore/protocol.h
Expand Up @@ -8,12 +8,6 @@ Restore Protocol Handler
#include "common/type/variantList.h"
#include "protocol/server.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_RESTORE_FILE "restoreFile"
STRING_DECLARE(PROTOCOL_COMMAND_RESTORE_FILE_STR);

/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
Expand All @@ -23,6 +17,8 @@ void restoreFileProtocol(const VariantList *paramList, ProtocolServer *server);
/***********************************************************************************************************************************
Protocol commands for ProtocolServerHandler arrays passed to protocolServerProcess()
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_RESTORE_FILE STRID5("rs-f", 0x36e720)

#define PROTOCOL_SERVER_HANDLER_RESTORE_LIST \
{.command = PROTOCOL_COMMAND_RESTORE_FILE, .handler = restoreFileProtocol},

Expand Down
2 changes: 1 addition & 1 deletion src/command/restore/restore.c
Expand Up @@ -2163,7 +2163,7 @@ static ProtocolParallelJob *restoreJobCallback(void *data, unsigned int clientId
const ManifestFile *file = *(ManifestFile **)lstGet(queue, 0);

// Create restore job
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_RESTORE_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_RESTORE_FILE);
protocolCommandParamAdd(command, VARSTR(file->name));
protocolCommandParamAdd(command, VARUINT(jobData->repoIdx));
protocolCommandParamAdd(
Expand Down
5 changes: 0 additions & 5 deletions src/command/verify/protocol.c
Expand Up @@ -12,11 +12,6 @@ Verify Protocol Handler
#include "config/config.h"
#include "storage/helper.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
STRING_EXTERN(PROTOCOL_COMMAND_VERIFY_FILE_STR, PROTOCOL_COMMAND_VERIFY_FILE);

/**********************************************************************************************************************************/
void
verifyFileProtocol(const VariantList *paramList, ProtocolServer *server)
Expand Down
8 changes: 2 additions & 6 deletions src/command/verify/protocol.h
Expand Up @@ -8,12 +8,6 @@ Verify Protocol Handler
#include "common/type/variantList.h"
#include "protocol/server.h"

/***********************************************************************************************************************************
Constants
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_VERIFY_FILE "verifyFile"
STRING_DECLARE(PROTOCOL_COMMAND_VERIFY_FILE_STR);

/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
Expand All @@ -23,6 +17,8 @@ void verifyFileProtocol(const VariantList *paramList, ProtocolServer *server);
/***********************************************************************************************************************************
Protocol commands for ProtocolServerHandler arrays passed to protocolServerProcess()
***********************************************************************************************************************************/
#define PROTOCOL_COMMAND_VERIFY_FILE STRID5("vf-f", 0x36cd60)

#define PROTOCOL_SERVER_HANDLER_VERIFY_LIST \
{.command = PROTOCOL_COMMAND_VERIFY_FILE, .handler = verifyFileProtocol},

Expand Down
4 changes: 2 additions & 2 deletions src/command/verify/verify.c
Expand Up @@ -754,7 +754,7 @@ verifyArchive(void *data)
String *checksum = strSubN(fileName, WAL_SEGMENT_NAME_SIZE + 1, HASH_TYPE_SHA1_SIZE_HEX);

// Set up the job
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_VERIFY_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_VERIFY_FILE);
protocolCommandParamAdd(command, VARSTR(filePathName));
protocolCommandParamAdd(command, VARSTR(checksum));
protocolCommandParamAdd(command, VARUINT64(archiveResult->pgWalInfo.size));
Expand Down Expand Up @@ -977,7 +977,7 @@ verifyBackup(void *data)
if (filePathName != NULL)
{
// Set up the job
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_VERIFY_FILE_STR);
ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_VERIFY_FILE);
protocolCommandParamAdd(command, VARSTR(filePathName));

// If the checksum is not present in the manifest, it will be calculated by manifest load
Expand Down

0 comments on commit ed0d48f

Please sign in to comment.