Merge pull request #1884 from Expensify/main

Update expensify_prod branch

.gitignore

@@ -16,3 +16,4 @@ libstuff/libstuff.h.gch
 .nfs*
 .cache
 compile_commands.json
+.vscode/*

BedrockServer.cpp

@@ -778,6 +778,7 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
     }
 
     int64_t lastConflictPage = 0;
+    string lastConflictTable;
     while (true) {
 
         // We just spin until the node looks ready to go. Typically, this doesn't happen expect briefly at startup.
@@ -1021,7 +1022,14 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
                         } else {
                             SINFO("Conflict or state change committing " << command->request.methodLine << " on worker thread.");
                             if (_enableConflictPageLocks) {
-                                lastConflictPage = db.getLastConflictPage();
+                                lastConflictTable = db.getLastConflictTable();
+
+                                // Journals are always chosen at the time of commit. So in case there was a conflict on the journal in 
+                                // the previous commit, the chances are very low (1/192) that we'll choose the same journal, thus, we
+                                // don't need to lock our next commit on this page conflict.
+                                if (!SStartsWith(lastConflictTable, "journal")) {
+                                    lastConflictPage = db.getLastConflictPage();
+                                }
                             }
                         }
                     } else if (result == BedrockCore::RESULT::NO_COMMIT_REQUIRED) {

sqlitecluster/SQLite.cpp

@@ -2,6 +2,7 @@
 
 #include <linux/limits.h>
 #include <string.h>
+#include <format>
 
 #include <libstuff/libstuff.h>
 #include <libstuff/SQResult.h>
@@ -17,6 +18,7 @@ sqlite3* SQLite::getDBHandle() {
 
 thread_local string SQLite::_mostRecentSQLiteErrorLog;
 thread_local int64_t SQLite::_conflictPage;
+thread_local string SQLite::_conflictTable;
 
 const string SQLite::getMostRecentSQLiteErrorLog() const {
     return _mostRecentSQLiteErrorLog;
@@ -282,8 +284,27 @@ void SQLite::_sqliteLogCallback(void* pArg, int iErrCode, const char* zMsg) {
     // This is sort of hacky to parse this from the logging info. If it works we could ask sqlite for a better interface to get this info.
     if (SStartsWith(zMsg, "cannot commit")) {
         // 17 is the length of "conflict at page" and the following space.
-        const char* offset = strstr(zMsg, "conflict at page") + 17;
-        _conflictPage = atol(offset);
+        const char* pageOffset = strstr(zMsg, "conflict at page") + 17;
+        _conflictPage = atol(pageOffset);
+
+        // Check if the tableOffset exists since not all conflicts are on tables
+        const char* tableOffset = strstr(pageOffset, "part of db table");
+        if (tableOffset) {
+            // 17 is the length of "part of db table" and the following space.
+            tableOffset += 17;
+
+            // Based on the SQLite log line, we should always have ';' after the table name,
+            // so let's find it and use it to limit the size of the substring we need
+            const char* semicolonOffset = strstr(tableOffset, ";");
+
+            // Let's add this check in case the SQLite log changes and we don't notice it
+            // since this would generate a runtime error.
+            if (semicolonOffset) {
+                _conflictTable = string(tableOffset, semicolonOffset - tableOffset);
+            } else {
+                _conflictTable = string(tableOffset);
+            }
+        }
     }
 }
 
@@ -401,6 +422,7 @@ bool SQLite::beginTransaction(TRANSACTION_TYPE type) {
     _commitElapsed = 0;
     _rollbackElapsed = 0;
     _lastConflictPage = 0;
+    _lastConflictTable = "";
     return _insideTransaction;
 }
 
@@ -726,12 +748,14 @@ int SQLite::commit(const string& description, function<void()>* preCheckpointCal
     sqlite3_db_status(_db, SQLITE_DBSTATUS_CACHE_WRITE, &startPages, &dummy, 0);
 
     _conflictPage = 0;
+    _conflictTable = "";
     uint64_t before = STimeNow();
     uint64_t beforeCommit = STimeNow();
     result = SQuery(_db, "committing db transaction", "COMMIT");
     _lastConflictPage = _conflictPage;
+    _lastConflictTable = _conflictTable;
     if (_lastConflictPage) {
-        SINFO("part of last conflict page: " << _lastConflictPage);
+        SINFO(format("part of last conflict page: {}, conflict table: {}",  _conflictPage, _conflictTable));
     }
 
     // If there were conflicting commits, will return SQLITE_BUSY_SNAPSHOT
@@ -789,6 +813,7 @@ int SQLite::commit(const string& description, function<void()>* preCheckpointCal
         _cacheHits = 0;
         _dbCountAtStart = 0;
         _lastConflictPage = 0;
+        _lastConflictTable = "";
     } else {
         SINFO("Commit failed, waiting for rollback.");
     }
@@ -1136,6 +1161,10 @@ int64_t SQLite::getLastConflictPage() const {
     return _lastConflictPage;
 }
 
+string SQLite::getLastConflictTable() const {
+    return _lastConflictTable;
+}
+
 SQLite::SharedData::SharedData() :
 nextJournalCount(0),
 _commitEnabled(true),

sqlitecluster/SQLite.h

@@ -248,6 +248,8 @@ class SQLite {
 
     int64_t getLastConflictPage() const;
 
+    string getLastConflictTable() const;
+
     // This is the callback function we use to log SQLite's internal errors.
     static void _sqliteLogCallback(void* pArg, int iErrCode, const char* zMsg);
 
@@ -398,7 +400,9 @@ class SQLite {
     bool _mutexLocked = false;
 
     atomic<int64_t> _lastConflictPage = 0;
+    atomic<string> _lastConflictTable;
     static thread_local int64_t _conflictPage;
+    static thread_local string _conflictTable;
 
     bool _writeIdempotent(const string& query, bool alwaysKeepQueries = false);