Merge pull request #1475 from Expensify/main

Update expensify_prod branch
Expensify · Mar 23, 2023 · 0f43b90 · 0f43b90
2 parents 3619b7c + f0c5e6a
commit 0f43b90
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 14 deletions.
diff --git a/sqlitecluster/SQLite.cpp b/sqlitecluster/SQLite.cpp
@@ -104,7 +104,10 @@ sqlite3* SQLite::initializeDB(const string& filename, int64_t mmapSizeGB, bool h
         // We only need to specify the full URL when creating new DBs. Existing DBs will be auto-detected as HC-Tree or not.
         completeFilename = "file://" + completeFilename + "?hctree=1";
     }
-    SASSERT(!sqlite3_open_v2(completeFilename.c_str(), &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX | SQLITE_OPEN_URI, NULL));
+    int result = sqlite3_open_v2(completeFilename.c_str(), &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX | SQLITE_OPEN_URI, NULL);
+    if (result) {
+        SERROR("sqlite3_open_v2 returned " << result << ", Extended error code: " << sqlite3_extended_errcode(db));
+    }
 
     // PRAGMA legacy_file_format=OFF sets the default for creating new databases, so it must be called before creating
     // any tables to be effective.

diff --git a/test/clustertest/tests/ForkCheckTest.cpp b/test/clustertest/tests/ForkCheckTest.cpp
@@ -9,13 +9,13 @@ struct ForkCheckTest : tpunit::TestFixture {
     ForkCheckTest()
         : tpunit::TestFixture("ForkCheck", TEST(ForkCheckTest::test)) {}
 
-    pair<uint64_t, string> getMaxJournalCommit(BedrockTester& tester) {
+    pair<uint64_t, string> getMaxJournalCommit(BedrockTester& tester, bool online = true) {
         SQResult journals;
-        tester.readDB("SELECT name FROM sqlite_schema WHERE type ='table' AND name LIKE 'journal%';", journals);
+        tester.readDB("SELECT name FROM sqlite_schema WHERE type ='table' AND name LIKE 'journal%';", journals, online);
         uint64_t maxJournalCommit = 0;
         string maxJournalTable;
         for (auto& row : journals.rows) {
-            string maxID = tester.readDB("SELECT MAX(id) FROM " + row[0] + ";");
+            string maxID = tester.readDB("SELECT MAX(id) FROM " + row[0] + ";", online);
             try {
                 uint64_t maxCommitNum = stoull(maxID);
                 if (maxCommitNum > maxJournalCommit) {
@@ -79,7 +79,8 @@ struct ForkCheckTest : tpunit::TestFixture {
         }
 
         // Break the journal on leader intentionally to fake a fork.
-        auto result = getMaxJournalCommit(tester.getTester(0));
+        auto result = getMaxJournalCommit(tester.getTester(0), false);
+
         uint64_t leaderMaxCommit = result.first;
         string leaderMaxCommitJournal = result.second;
         result = getMaxJournalCommit(tester.getTester(1));
@@ -88,6 +89,9 @@ struct ForkCheckTest : tpunit::TestFixture {
         // Make sure the follower got farther than the leader.
         ASSERT_GREATER_THAN(followerMaxCommit, leaderMaxCommit);
 
+        // We need to release any DB that the tester is holding.
+        tester.getTester(0).freeDB();
+
         // Break leader.
         {
             string filename = tester.getTester(0).getArg("-db");
@@ -107,7 +111,7 @@ struct ForkCheckTest : tpunit::TestFixture {
         tester.getTester(0).startServer(false);
 
         // We expect it to die shortly.
-        int status;
+        int status = 0;
         waitpid(tester.getTester(0).getPID(), &status, 0);
 
         // Should have gotten a signal when it died.

diff --git a/test/lib/BedrockTester.cpp b/test/lib/BedrockTester.cpp
@@ -367,7 +367,7 @@ vector<SData> BedrockTester::executeWaitMultipleData(vector<SData> requests, int
                                 *errorCode = 2;
                             }
                             if (timeout) {
-                                cout << "executeWaitMultiple(): ran out of time waiting for socket" << endl;
+                                cout << "executeWaitMultipleData(): ran out of time waiting for socket" << endl;
                             }
                             return;
                         }
@@ -525,18 +525,27 @@ SQLite& BedrockTester::getSQLiteDB()
 {
     if (!_db) {
         // Assumes wal2 mode.
-        _db = new SQLite(_args["-db"], 1000000, 3000000, -1, "", 0);
+        _db = new SQLite(_args["-db"], 1000000, 3000000, -1, "", 0, ENABLE_HCTREE);
     }
     return *_db;
 }
 
-string BedrockTester::readDB(const string& query)
+void BedrockTester::freeDB() {
+    delete _db;
+    _db = nullptr;
+}
+
+string BedrockTester::readDB(const string& query, bool online)
 {
-    if (ENABLE_HCTREE) {
+    if (ENABLE_HCTREE && online) {
         SData command("Query");
         command["Query"] = query;
         command["Format"] = "JSON";
         auto row0 = SParseJSONObject(executeWaitMultipleData({command})[0].content)["rows"];
+        if (row0 == "") {
+            return "";
+        }
+
         return SParseJSONArray(SParseJSONArray(row0).front()).front();
     } else {
         SQLite& db = getSQLiteDB();
@@ -547,9 +556,9 @@ string BedrockTester::readDB(const string& query)
     }
 }
 
-bool BedrockTester::readDB(const string& query, SQResult& result)
+bool BedrockTester::readDB(const string& query, SQResult& result, bool online)
 {
-    if (ENABLE_HCTREE) {
+    if (ENABLE_HCTREE && online) {
         result.clear();
         SData command("Query");
         command["Query"] = query;

diff --git a/test/lib/BedrockTester.h b/test/lib/BedrockTester.h
@@ -74,8 +74,11 @@ class BedrockTester {
 
     // Read from the DB file, without going through the bedrock server. Two interfaces are provided to maintain
     // compatibility with the `SQLite` class.
-    string readDB(const string& query);
-    bool readDB(const string& query, SQResult& result);
+    string readDB(const string& query, bool online = true);
+    bool readDB(const string& query, SQResult& result, bool online = true);
+
+    // Closes and releases any existing DB file.
+    void freeDB();
 
     // Wait for a particular key in a `Status` message to equal a particular value, for up to `timeoutUS` us. Returns
     // true if a match was found, or times out otherwose.