Merge pull request #764 from Expensify/joel_addEscalatedParam

Adding an escalated parameter and updating it in escalateCommand
Expensify · Apr 23, 2020 · e5057f3 · e5057f3
2 parents fb370a0 + 4093396
commit e5057f3
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 4 deletions.
diff --git a/sqlitecluster/SQLiteCommand.cpp b/sqlitecluster/SQLiteCommand.cpp
@@ -26,7 +26,8 @@ SQLiteCommand::SQLiteCommand(SData&& _request) :
     writeConsistency(SQLiteNode::ASYNC),
     complete(false),
     escalationTimeUS(0),
-    creationTime(STimeNow())
+    creationTime(STimeNow()),
+    escalated(false)
 {
     // Initialize the consistency, if supplied.
     if (request.isSet("writeConsistency")) {
@@ -53,5 +54,6 @@ SQLiteCommand::SQLiteCommand() :
     writeConsistency(SQLiteNode::ASYNC),
     complete(false),
     escalationTimeUS(0),
-    creationTime(STimeNow())
+    creationTime(STimeNow()),
+    escalated(false)
 { }
diff --git a/sqlitecluster/SQLiteCommand.h b/sqlitecluster/SQLiteCommand.h
@@ -56,6 +56,9 @@ class SQLiteCommand {
     // follower to leader.
     uint64_t creationTime;
 
+    // Whether or not the command has been escalated.
+    bool escalated;
+
     // Construct that takes a request object.
     SQLiteCommand(SData&& _request);
 

diff --git a/sqlitecluster/SQLiteNode.cpp b/sqlitecluster/SQLiteNode.cpp
@@ -249,6 +249,10 @@ void SQLiteNode::escalateCommand(unique_ptr<SQLiteCommand>&& command, bool forge
     escalate["ID"] = command->id;
     escalate.content = command->request.serialize();
 
+    // Marking the command as escalated, even if we are going to forget it, because the command's destructor may need
+    // this info.
+    command->escalated = true;
+
     // Store the command as escalated, unless we intend to forget about it anyway.
     if (forget) {
         SINFO("Firing and forgetting command '" << command->request.methodLine << "' to leader.");

diff --git a/test/clustertest/testplugin/TestPlugin.cpp b/test/clustertest/testplugin/TestPlugin.cpp
@@ -115,6 +115,7 @@ TestPluginCommand::~TestPluginCommand()
         // check simply that we're not leading, because this should also fail if we end up in some weird state (we
         // don't want the test to pass if our follower is actually `WAITING` or something strange).
         if (serverState != SQLiteNode::stateName(SQLiteNode::LEADING)) {
+            SASSERT(escalated);
             string fileContents = fileLockAndLoad(request["tempFile"]);
             SFileDelete(request["tempFile"]);
 
@@ -356,8 +357,7 @@ void TestPluginCommand::process(SQLite& db) {
 
         // Done.
         return;
-    }
-    else if (SStartsWith(request.methodLine, "idcollision")) {
+    } else if (SStartsWith(request.methodLine, "idcollision")) {
         SQResult result;
         db.read("SELECT MAX(id) FROM test", result);
         SASSERT(result.size());

diff --git a/test/clustertest/tests/EscalateTest.cpp b/test/clustertest/tests/EscalateTest.cpp
@@ -18,6 +18,10 @@ struct EscalateTest : tpunit::TestFixture {
         cmd["tempFile"] = BedrockTester::getTempFileName("escalate_test");
         brtester.executeWaitMultipleData({cmd});
 
+        // Sleep for 1 second to make sure, if there was a crash, that the next command does not run before the server
+        // fully crashes.
+        sleep(1);
+
         // Because the way the above escalation is verified is in the destructor for the command, we send another request to
         // verify the server hasn't crashed.
         SData status("Status");