Merge pull request #1963 from Expensify/main

Update expensify_prod branch

BedrockServer.cpp

@@ -97,7 +97,7 @@ void BedrockServer::sync()
     // We use fewer FDs on test machines that have other resource restrictions in place.
 
     SINFO("Setting dbPool size to: " << _dbPoolSize);
-    _dbPool = make_shared<SQLitePool>(_dbPoolSize, args["-db"], args.calc("-cacheSize"), args.calc("-maxJournalSize"), journalTables, args["-synchronous"], mmapSizeGB, args.isSet("-hctree"));
+    _dbPool = make_shared<SQLitePool>(_dbPoolSize, args["-db"], args.calc("-cacheSize"), args.calc("-maxJournalSize"), journalTables, mmapSizeGB, args.isSet("-hctree"));
     SQLite& db = _dbPool->getBase();
 
     // Initialize the command processor.
@@ -1026,7 +1026,7 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
                             if (_enableConflictPageLocks) {
                                 lastConflictTable = db.getLastConflictTable();
 
-                                // Journals are always chosen at the time of commit. So in case there was a conflict on the journal in 
+                                // 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")) {
@@ -1671,11 +1671,25 @@ void BedrockServer::_status(unique_ptr<BedrockCommand>& command) {
         {
             // Make it known if anything is known to cause crashes.
             shared_lock<decltype(_crashCommandMutex)> lock(_crashCommandMutex);
+            vector<string> crashCommandListArray;
+
             size_t totalCount = 0;
             for (const auto& s : _crashCommands) {
                 totalCount += s.second.size();
+
+                vector<string> paramsArray;
+                for (const STable& params : s.second) {
+                    if (!params.empty()) {
+                        paramsArray.push_back(SComposeJSONObject(params));
+                    }
+                }
+
+                STable commandObject;
+                commandObject[s.first] = SComposeJSONArray(paramsArray);
+                crashCommandListArray.push_back(SComposeJSONObject(commandObject));
             }
             content["crashCommands"] = totalCount;
+            content["crashCommandList"] = SComposeJSONArray(crashCommandListArray);
         }
 
         // On leader, return the current multi-write blacklists.

docker/README.md

@@ -10,7 +10,7 @@ This folder contains the necessary files to build and run Bedrock in a Docker co
 
 ## Dockerfile
 
-The Dockerfile sets up a container based on [`phusion/baseimage:noble-1.0.0`](https://github.com/phusion/baseimage-docker) 
+The Dockerfile sets up a container based on [`phusion/baseimage:noble-1.0.0`](https://github.com/phusion/baseimage-docker)
 and includes the following key steps:
 
 1. Installs necessary packages
@@ -90,7 +90,6 @@ The following environment variables can be used to configure Bedrock:
 - `WORKER_THREADS`: Number of worker threads
 - `QUERY_LOG`: Query log configuration
 - `MAX_JOURNAL_SIZE`: Maximum journal size
-- `SYNCHRONOUS`: Synchronous mode setting
 
 Additional flags can be set using the `VERBOSE`, `QUIET`, and `CLEAN` environment variables.
 
@@ -118,4 +117,4 @@ bedrock-node2  | Oct 17 16:14:52 da62315a18ec bedrock: xxxxxx (SQLiteNode.cpp:12
 
 ## Network
 
-The Bedrock nodes are connected to a custom bridge network `bedrock-cluster`, allowing for easy communication between nodes.
+The Bedrock nodes are connected to a custom bridge network `bedrock-cluster`, allowing for easy communication between nodes.

main.cpp

@@ -236,9 +236,6 @@ int main(int argc, char* argv[]) {
              << endl;
         cout << "-maxJournalSize <#commits>  Number of commits to retain in the historical journal (default 1000000)"
              << endl;
-        cout << "-synchronous    <value>     Set the PRAGMA schema.synchronous "
-                "(defaults see https://sqlite.org/pragma.html#pragma_synchronous)"
-             << endl;
         cout << endl;
         cout << "Quick Start Tips:" << endl;
         cout << "-----------------" << endl;

sqlitecluster/SQLite.cpp

@@ -218,24 +218,19 @@ void SQLite::commonConstructorInitialization(bool hctree) {
     // Setting a wal hook prevents auto-checkpointing.
     sqlite3_wal_hook(_db, _walHookCallback, this);
 
-    // Check if synchronous has been set and run query to use a custom synchronous setting
-    if (!_synchronous.empty()) {
-        SASSERT(!SQuery(_db, "setting custom synchronous commits", "PRAGMA synchronous = " + SQ(_synchronous)  + ";"));
-    } else {
-        DBINFO("Using SQLite default PRAGMA synchronous");
-    }
+    // Always set synchronous commits to off for best commit performance in WAL mode.
+    SASSERT(!SQuery(_db, "setting synchronous commits to off", "PRAGMA synchronous = OFF;"));
 }
 
 SQLite::SQLite(const string& filename, int cacheSize, int maxJournalSize,
-               int minJournalTables, const string& synchronous, int64_t mmapSizeGB, bool hctree) :
+               int minJournalTables, int64_t mmapSizeGB, bool hctree) :
     _filename(initializeFilename(filename)),
     _maxJournalSize(maxJournalSize),
     _db(initializeDB(_filename, mmapSizeGB, hctree)),
     _journalNames(initializeJournal(_db, minJournalTables)),
     _sharedData(initializeSharedData(_db, _filename, _journalNames, hctree)),
     _journalSize(initializeJournalSize(_db, _journalNames)),
     _cacheSize(cacheSize),
-    _synchronous(synchronous),
     _mmapSizeGB(mmapSizeGB)
 {
     commonConstructorInitialization(hctree);
@@ -249,7 +244,6 @@ SQLite::SQLite(const SQLite& from) :
     _sharedData(from._sharedData),
     _journalSize(from._journalSize),
     _cacheSize(from._cacheSize),
-    _synchronous(from._synchronous),
     _mmapSizeGB(from._mmapSizeGB)
 {
     // This can always pass "true" because the copy constructor does not need to set the DB to WAL2 mode, it would have been set in the object being copied.

sqlitecluster/SQLite.h

@@ -56,12 +56,8 @@ class SQLite {
     //
     // mmapSizeGB: address space to use for memory-mapped IO, in GB.
     SQLite(const string& filename, int cacheSize, int maxJournalSize, int minJournalTables,
-           const string& synchronous = "", int64_t mmapSizeGB = 0, bool hctree = false);
+           int64_t mmapSizeGB = 0, bool hctree = false);
 
-    // Compatibility constructor. Remove when AuthTester::getStripeSQLiteDB no longer uses this outdated version.
-    SQLite(const string& filename, int cacheSize, int maxJournalSize, int minJournalTables, int synchronous) :
-        SQLite(filename, cacheSize, maxJournalSize, minJournalTables, "") {}
-
     // This constructor is not exactly a copy constructor. It creates an other SQLite object based on the first except
     // with a *different* journal table. This avoids a lot of locking around creating structures that we know already
     // exist because we already have a SQLite object for this file.
@@ -154,18 +150,18 @@ class SQLite {
     void setRewriteHandler(bool (*handler)(int, const char*, string&));
 
     // Enables the on prepare handler.
-    // The on commit handler allows a plugin to be notified when a transaction is prepared but not yet committed. 
+    // The on commit handler allows a plugin to be notified when a transaction is prepared but not yet committed.
     // This allows the plugin to take arbitrary actions prior to committing to the database. Bedrock does not
     // pass up any information in this case, it simply notifies the plugin that a transaction was prepared.
     void enablePrepareNotifications(bool enable);
 
     // Update the on prepare handler.
     // The on prepare handler accepts a reference to this SQLiteDB object and an int tableID. The tableID is the
     // same ID that is used for the journal number in the current running thread. This allows the handler to utilize
-    // SQLite.cpp's method for avoiding conflicts on tables written on every command. 
+    // SQLite.cpp's method for avoiding conflicts on tables written on every command.
     // IMPORTANT: The on prepare handler allows a plugin to run code inside the commit lock. This code should be time sensitive
     // as increases to the amount of time this lock is held increase conflict chances and decreases the parallelness
-    // of bedrock commands. 
+    // of bedrock commands.
     // IMPORTANT: there can be only one on-prepare handler for a given DB at once.
     void setOnPrepareHandler(void (*handler)(SQLite& _db, int64_t tableID));
 
@@ -505,7 +501,6 @@ class SQLite {
 
     // Copies of parameters used to initialize the DB that we store if we make child objects based on this one.
     int _cacheSize;
-    const string _synchronous;
     int64_t _mmapSizeGB;
 
     // This is a string (which may be empty) containing the most recent logged error by SQLite in this thread.

sqlitecluster/SQLitePool.cpp

@@ -7,11 +7,10 @@ SQLitePool::SQLitePool(size_t maxDBs,
                        int cacheSize,
                        int maxJournalSize,
                        int minJournalTables,
-                       const string& synchronous,
                        int64_t mmapSizeGB,
                        bool hctree)
 : _maxDBs(max(maxDBs, 1ul)),
-  _baseDB(filename, cacheSize, maxJournalSize, minJournalTables, synchronous, mmapSizeGB, hctree),
+  _baseDB(filename, cacheSize, maxJournalSize, minJournalTables, mmapSizeGB, hctree),
   _objects(_maxDBs, nullptr)
 {
 }

sqlitecluster/SQLitePool.h

@@ -7,7 +7,7 @@ class SQLitePool {
   public:
     // Create a pool of DB handles.
     SQLitePool(size_t maxDBs, const string& filename, int cacheSize, int maxJournalSize, int minJournalTables,
-               const string& synchronous = "", int64_t mmapSizeGB = 0, bool hctree = false);
+               int64_t mmapSizeGB = 0, bool hctree = false);
     ~SQLitePool();
 
     // Get the base object (the first one created, which uses the `journal` table). Note that if called by multiple

test/lib/BedrockTester.cpp

@@ -536,7 +536,7 @@ SQLite& BedrockTester::getSQLiteDB()
 {
     if (!_db) {
         // Assumes wal2 mode.
-        _db = new SQLite(_args["-db"], 1000000, 3000000, -1, "", 0, ENABLE_HCTREE);
+        _db = new SQLite(_args["-db"], 1000000, 3000000, -1, 0, ENABLE_HCTREE);
     }
     return *_db;
 }