From b3277f2982e6452e667df2b17bf7e19980cc97f2 Mon Sep 17 00:00:00 2001 From: sramamoorthy Date: Wed, 28 Aug 2019 10:52:56 -0700 Subject: [PATCH] Fix #2009 posix compliant args for snapshot binary --- fdbcli/fdbcli.actor.cpp | 13 ++++++++--- fdbclient/ManagementAPI.actor.cpp | 2 +- fdbclient/ManagementAPI.actor.h | 2 +- fdbclient/NativeAPI.actor.cpp | 14 ++---------- fdbclient/NativeAPI.actor.h | 2 +- fdbserver/FDBExecHelper.actor.cpp | 32 +++++++++------------------ fdbserver/FDBExecHelper.actor.h | 4 +--- fdbserver/OldTLogServer_6_0.actor.cpp | 3 +-- fdbserver/TLogServer.actor.cpp | 3 +-- fdbserver/worker.actor.cpp | 3 +-- 10 files changed, 30 insertions(+), 48 deletions(-) diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index 88fa6cb4f7..2d301a135f 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -2199,7 +2199,14 @@ ACTOR Future exclude( Database db, std::vector tokens, Referenc } } -ACTOR Future createSnapshot(Database db, StringRef snapCmd) { +ACTOR Future createSnapshot(Database db, std::vector tokens ) { + state Standalone snapCmd; + for ( int i = 1; i < tokens.size(); i++) { + snapCmd = snapCmd.withSuffix(tokens[i]); + if (i != tokens.size() - 1) { + snapCmd = snapCmd.withSuffix(LiteralStringRef(" ")); + } + } try { UID snapUID = wait(makeInterruptable(mgmtSnapCreate(db, snapCmd))); printf("Snapshot command succeeded with UID %s\n", snapUID.toString().c_str()); @@ -2815,11 +2822,11 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { } if (tokencmp(tokens[0], "snapshot")) { - if (tokens.size() != 2) { + if (tokens.size() < 2) { printUsage(tokens[0]); is_error = true; } else { - bool err = wait(createSnapshot(db, tokens[1])); + bool err = wait(createSnapshot(db, tokens)); if (err) is_error = true; } continue; diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index d7e40e1a82..5f5dc89311 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -1576,7 +1576,7 @@ ACTOR Future> checkForExcludingServers(Database cx, vec return inProgressExclusion; } -ACTOR Future mgmtSnapCreate(Database cx, StringRef snapCmd) { +ACTOR Future mgmtSnapCreate(Database cx, Standalone snapCmd) { state UID snapUID = deterministicRandom()->randomUniqueID(); try { wait(snapCreate(cx, snapCmd, snapUID)); diff --git a/fdbclient/ManagementAPI.actor.h b/fdbclient/ManagementAPI.actor.h index 5e66f9d02c..f3aabec8fe 100644 --- a/fdbclient/ManagementAPI.actor.h +++ b/fdbclient/ManagementAPI.actor.h @@ -197,7 +197,7 @@ bool schemaMatch( json_spirit::mValue const& schema, json_spirit::mValue const& // execute payload in 'snapCmd' on all the coordinators, TLogs and // storage nodes -ACTOR Future mgmtSnapCreate(Database cx, StringRef snapCmd); +ACTOR Future mgmtSnapCreate(Database cx, Standalone snapCmd); #include "flow/unactorcompiler.h" #endif diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 20c5509b38..a2894cfdf1 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -3348,25 +3348,15 @@ void enableClientInfoLogging() { TraceEvent(SevInfo, "ClientInfoLoggingEnabled"); } -ACTOR Future snapCreate(Database cx, StringRef snapCmd, UID snapUID) { +ACTOR Future snapCreate(Database cx, Standalone snapCmd, UID snapUID) { TraceEvent("SnapCreateEnter") .detail("SnapCmd", snapCmd.toString()) .detail("UID", snapUID); - - StringRef snapCmdArgs = snapCmd; - StringRef snapCmdPart = snapCmdArgs.eat(":"); - Standalone snapUIDRef(snapUID.toString()); - state Standalone snapPayloadRef = snapCmdPart - .withSuffix(LiteralStringRef(":uid=")) - .withSuffix(snapUIDRef) - .withSuffix(LiteralStringRef(",")) - .withSuffix(snapCmdArgs); - try { loop { choose { when(wait(cx->onMasterProxiesChanged())) {} - when(wait(loadBalance(cx->getMasterProxies(false), &MasterProxyInterface::proxySnapReq, ProxySnapRequest(snapPayloadRef, snapUID, snapUID), cx->taskID, true /*atmostOnce*/ ))) { + when(wait(loadBalance(cx->getMasterProxies(false), &MasterProxyInterface::proxySnapReq, ProxySnapRequest(snapCmd, snapUID, snapUID), cx->taskID, true /*atmostOnce*/ ))) { TraceEvent("SnapCreateExit") .detail("SnapCmd", snapCmd.toString()) .detail("UID", snapUID); diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h index 58226826d1..716a0cad89 100644 --- a/fdbclient/NativeAPI.actor.h +++ b/fdbclient/NativeAPI.actor.h @@ -310,7 +310,7 @@ int64_t extractIntOption( Optional value, int64_t minValue = std::num // Takes a snapshot of the cluster, specifically the following persistent // states: coordinator, TLog and storage state -ACTOR Future snapCreate(Database cx, StringRef snapCmd, UID snapUID); +ACTOR Future snapCreate(Database cx, Standalone snapCmd, UID snapUID); #include "flow/unactorcompiler.h" #endif diff --git a/fdbserver/FDBExecHelper.actor.cpp b/fdbserver/FDBExecHelper.actor.cpp index a207bd5c90..d987930a17 100644 --- a/fdbserver/FDBExecHelper.actor.cpp +++ b/fdbserver/FDBExecHelper.actor.cpp @@ -21,7 +21,6 @@ ExecCmdValueString::ExecCmdValueString(StringRef pCmdValueString) { void ExecCmdValueString::setCmdValueString(StringRef pCmdValueString) { // reset everything binaryPath = StringRef(); - keyValueMap.clear(); // set the new cmdValueString cmdValueString = pCmdValueString; @@ -42,18 +41,10 @@ VectorRef ExecCmdValueString::getBinaryArgs() { return binaryArgs; } -StringRef ExecCmdValueString::getBinaryArgValue(StringRef key) { - StringRef res; - if (keyValueMap.find(key) != keyValueMap.end()) { - res = keyValueMap[key]; - } - return res; -} - void ExecCmdValueString::parseCmdValue() { StringRef param = this->cmdValueString; // get the binary path - this->binaryPath = param.eat(LiteralStringRef(":")); + this->binaryPath = param.eat(LiteralStringRef(" ")); // no arguments provided if (param == StringRef()) { @@ -62,11 +53,8 @@ void ExecCmdValueString::parseCmdValue() { // extract the arguments while (param != StringRef()) { - StringRef token = param.eat(LiteralStringRef(",")); + StringRef token = param.eat(LiteralStringRef(" ")); this->binaryArgs.push_back(this->binaryArgs.arena(), token); - - StringRef key = token.eat(LiteralStringRef("=")); - keyValueMap.insert(std::make_pair(key, token)); } return; } @@ -153,15 +141,14 @@ ACTOR Future spawnProcess(std::string binPath, std::vector par } #endif -ACTOR Future execHelper(ExecCmdValueString* execArg, std::string folder, std::string role) { - state StringRef uidStr = execArg->getBinaryArgValue(LiteralStringRef("uid")); +ACTOR Future execHelper(ExecCmdValueString* execArg, UID snapUID, std::string folder, std::string role) { + state Standalone uidStr = snapUID.toString(); state int err = 0; state Future cmdErr; state double maxWaitTime = SERVER_KNOBS->SNAP_CREATE_MAX_TIMEOUT; if (!g_network->isSimulated()) { // get bin path auto snapBin = execArg->getBinaryPath(); - auto dataFolder = "path=" + folder; std::vector paramList; paramList.push_back(snapBin.toString()); // get user passed arguments @@ -170,12 +157,15 @@ ACTOR Future execHelper(ExecCmdValueString* execArg, std::string folder, st paramList.push_back(elem.toString()); } // get additional arguments - paramList.push_back(dataFolder); + paramList.push_back("--path"); + paramList.push_back(folder); const char* version = FDB_VT_VERSION; - std::string versionString = "version="; - versionString += version; - paramList.push_back(versionString); + paramList.push_back("--version"); + paramList.push_back(version); + paramList.push_back("--role"); paramList.push_back(role); + paramList.push_back("--uid"); + paramList.push_back(uidStr.toString()); cmdErr = spawnProcess(snapBin.toString(), paramList, maxWaitTime, false /*isSync*/, 0); wait(success(cmdErr)); err = cmdErr.get(); diff --git a/fdbserver/FDBExecHelper.actor.h b/fdbserver/FDBExecHelper.actor.h index 49792e1949..5e064218ab 100644 --- a/fdbserver/FDBExecHelper.actor.h +++ b/fdbserver/FDBExecHelper.actor.h @@ -27,7 +27,6 @@ public: // ctor & dtor public: // interfaces StringRef getBinaryPath(); VectorRef getBinaryArgs(); - StringRef getBinaryArgValue(StringRef key); void setCmdValueString(StringRef cmdValueString); StringRef getCmdValueString(void); @@ -41,7 +40,6 @@ private: // data Standalone cmdValueString; Standalone> binaryArgs; StringRef binaryPath; - std::map keyValueMap; }; // FIXME: move this function to a common location @@ -52,7 +50,7 @@ private: // data ACTOR Future spawnProcess(std::string binPath, std::vector paramList, double maxWaitTime, bool isSync, double maxSimDelayTime); // helper to run all the work related to running the exec command -ACTOR Future execHelper(ExecCmdValueString* execArg, std::string folder, std::string role); +ACTOR Future execHelper(ExecCmdValueString* execArg, UID snapUID, std::string folder, std::string role); // returns true if the execUID op is in progress bool isExecOpInProgress(UID execUID); diff --git a/fdbserver/OldTLogServer_6_0.actor.cpp b/fdbserver/OldTLogServer_6_0.actor.cpp index 10f191b937..acd16fb649 100644 --- a/fdbserver/OldTLogServer_6_0.actor.cpp +++ b/fdbserver/OldTLogServer_6_0.actor.cpp @@ -1556,8 +1556,7 @@ tLogSnapCreate(TLogSnapRequest snapReq, TLogData* self, Reference logDa } ExecCmdValueString snapArg(snapReq.snapPayload); try { - Standalone role = LiteralStringRef("role=").withSuffix(snapReq.role); - int err = wait(execHelper(&snapArg, self->dataFolder, role.toString())); + int err = wait(execHelper(&snapArg, snapReq.snapUID, self->dataFolder, snapReq.role.toString())); std::string uidStr = snapReq.snapUID.toString(); TraceEvent("ExecTraceTLog") diff --git a/fdbserver/TLogServer.actor.cpp b/fdbserver/TLogServer.actor.cpp index 95d51267c5..c4701f464a 100644 --- a/fdbserver/TLogServer.actor.cpp +++ b/fdbserver/TLogServer.actor.cpp @@ -1933,8 +1933,7 @@ tLogSnapCreate(TLogSnapRequest snapReq, TLogData* self, Reference logDa } ExecCmdValueString snapArg(snapReq.snapPayload); try { - Standalone role = LiteralStringRef("role=").withSuffix(snapReq.role); - int err = wait(execHelper(&snapArg, self->dataFolder, role.toString())); + int err = wait(execHelper(&snapArg, snapReq.snapUID, self->dataFolder, snapReq.role.toString())); std::string uidStr = snapReq.snapUID.toString(); TraceEvent("ExecTraceTLog") diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index 622e197f2b..e2ab425f52 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -658,8 +658,7 @@ void endRole(const Role &role, UID id, std::string reason, bool ok, Error e) { ACTOR Future workerSnapCreate(WorkerSnapRequest snapReq, StringRef snapFolder) { state ExecCmdValueString snapArg(snapReq.snapPayload); try { - Standalone role = LiteralStringRef("role=").withSuffix(snapReq.role); - int err = wait(execHelper(&snapArg, snapFolder.toString(), role.toString())); + int err = wait(execHelper(&snapArg, snapReq.snapUID, snapFolder.toString(), snapReq.role.toString())); std::string uidStr = snapReq.snapUID.toString(); TraceEvent("ExecTraceWorker") .detail("Uid", uidStr)