Fix Makefile conflicts

Author: flodnv

.github/workflows/bedrock.yml

@@ -21,7 +21,7 @@ jobs:
 
     - name: Install the Mold Linker
       uses: rui314/setup-mold@v1
-      
+
     - name: Checkout Bedrock
       uses: actions/[email protected]
 
@@ -32,7 +32,7 @@ jobs:
       shell: bash
 
     - name: Set up cache
-      uses: actions/cache@v4.0.0
+      uses: actions/cache@v4.2.0
       with:
         path: |-
           ${{ env.CCACHE_BASEDIR }}

BedrockCommand.cpp

@@ -57,6 +57,10 @@ const string& BedrockCommand::getName() const {
     return defaultPluginName;
 }
 
+const BedrockPlugin* BedrockCommand::getPlugin() const {
+    return _plugin;
+}
+
 int64_t BedrockCommand::_getTimeout(const SData& request, const uint64_t scheduledTime) {
     // Timeout is the default, unless explicitly supplied, or if Connection: forget is set.
     int64_t timeout =  DEFAULT_TIMEOUT;

BedrockCommand.h

@@ -85,6 +85,8 @@ class BedrockCommand : public SQLiteCommand {
     // Return the name of the plugin for this command.
     const string& getName() const;
 
+    const BedrockPlugin* getPlugin() const;
+
     // Take all of the HTTPS requests attached to this object, and serialize them to a string.
     string serializeHTTPSRequests();
 
@@ -99,7 +101,7 @@ class BedrockCommand : public SQLiteCommand {
     }
 
     // Bedrock will call this before writing to the database after it has prepared a transaction for each plugin to allow it to
-    // enable a handler function for prepare If a plugin would like to perform operations after prepare but before commit, this should 
+    // enable a handler function for prepare If a plugin would like to perform operations after prepare but before commit, this should
     // return true, and it should set the prepareHandler it would like to use.
     virtual bool shouldEnableOnPrepareNotification(const SQLite& db, void (**onPrepareHandler)(SQLite& _db, int64_t tableID)) {
         return false;

BedrockCore.cpp

@@ -52,12 +52,12 @@ uint64_t BedrockCore::_getRemainingTime(const unique_ptr<BedrockCommand>& comman
     return isProcessing ? min(processTimeout, adjustedTimeout) : adjustedTimeout;
 }
 
-bool BedrockCore::isTimedOut(unique_ptr<BedrockCommand>& command) {
+bool BedrockCore::isTimedOut(unique_ptr<BedrockCommand>& command, SQLite* db, const BedrockServer* server) {
     try {
         _getRemainingTime(command, false);
     } catch (const SException& e) {
         // Yep, timed out.
-        _handleCommandException(command, e);
+        _handleCommandException(command, e, db, server);
         command->complete = true;
         return true;
     }
@@ -104,7 +104,7 @@ void BedrockCore::prePeekCommand(unique_ptr<BedrockCommand>& command, bool isBlo
             STHROW("555 Timeout prePeeking command");
         }
     } catch (const SException& e) {
-        _handleCommandException(command, e);
+        _handleCommandException(command, e, &_db, &_server);
         command->complete = true;
     } catch (...) {
         SALERT("Unhandled exception typename: " << SGetCurrentExceptionName() << ", command: " << request.methodLine);
@@ -186,7 +186,7 @@ BedrockCore::RESULT BedrockCore::peekCommand(unique_ptr<BedrockCommand>& command
         }
     } catch (const SException& e) {
         command->repeek = false;
-        _handleCommandException(command, e);
+        _handleCommandException(command, e, &_db, &_server);
     } catch (const SHTTPSManager::NotLeading& e) {
         command->repeek = false;
         returnValue = RESULT::SHOULD_PROCESS;
@@ -284,7 +284,7 @@ BedrockCore::RESULT BedrockCore::processCommand(unique_ptr<BedrockCommand>& comm
             }
         }
     } catch (const SException& e) {
-        _handleCommandException(command, e);
+        _handleCommandException(command, e, &_db, &_server);
         _db.rollback();
         needsCommit = false;
     } catch (const SQLite::constraint_error& e) {
@@ -353,7 +353,7 @@ void BedrockCore::postProcessCommand(unique_ptr<BedrockCommand>& command, bool i
             STHROW("555 Timeout postProcessing command");
         }
     } catch (const SException& e) {
-        _handleCommandException(command, e);
+        _handleCommandException(command, e, &_db, &_server);
     } catch (...) {
         SALERT("Unhandled exception typename: " << SGetCurrentExceptionName() << ", command: " << request.methodLine);
         command->response.methodLine = "500 Unhandled Exception";
@@ -367,7 +367,7 @@ void BedrockCore::postProcessCommand(unique_ptr<BedrockCommand>& command, bool i
     _db.setQueryOnly(false);
 }
 
-void BedrockCore::_handleCommandException(unique_ptr<BedrockCommand>& command, const SException& e) {
+void BedrockCore::_handleCommandException(unique_ptr<BedrockCommand>& command, const SException& e, SQLite* db, const BedrockServer* server) {
     string msg = "Error processing command '" + command->request.methodLine + "' (" + e.what() + "), ignoring.";
     if (!e.body.empty()) {
         msg = msg + " Request body: " + e.body;
@@ -396,9 +396,11 @@ void BedrockCore::_handleCommandException(unique_ptr<BedrockCommand>& command, c
     }
 
     // Add the commitCount header to the response.
-    command->response["commitCount"] = to_string(_db.getCommitCount());
+    if (db) {
+        command->response["commitCount"] = to_string(db->getCommitCount());
+    }
 
-    if (_server.args.isSet("-extraExceptionLogging")) {
+    if (server && server->args.isSet("-extraExceptionLogging")) {
         auto stack = e.details();
         command->response["exceptionSource"] = stack.back();
     }

BedrockCore.h

@@ -34,7 +34,7 @@ class BedrockCore : public SQLiteCore {
     // Checks if a command has already timed out. Like `peekCommand` without doing any work. Returns `true` and sets
     // the same command state as `peekCommand` would if the command has timed out. Returns `false` and does nothing if
     // the command hasn't timed out.
-    bool isTimedOut(unique_ptr<BedrockCommand>& command);
+    static bool isTimedOut(unique_ptr<BedrockCommand>& command, SQLite* db = nullptr, const BedrockServer* server = nullptr);
 
     void prePeekCommand(unique_ptr<BedrockCommand>& command, bool isBlockingCommitThread);
 
@@ -71,8 +71,8 @@ class BedrockCore : public SQLiteCore {
     // Gets the amount of time remaining until this command times out. This is the difference between the command's
     // 'timeout' value (or the default timeout, if not set) and the time the command was initially scheduled to run. If
     // this time is already expired, this throws `555 Timeout`
-    uint64_t _getRemainingTime(const unique_ptr<BedrockCommand>& command, bool isProcessing);
+    static uint64_t _getRemainingTime(const unique_ptr<BedrockCommand>& command, bool isProcessing);
 
-    void _handleCommandException(unique_ptr<BedrockCommand>& command, const SException& e);
+    static void _handleCommandException(unique_ptr<BedrockCommand>& command, const SException& e, SQLite* db = nullptr, const BedrockServer* server = nullptr);
     const BedrockServer& _server;
 };

BedrockPlugin.cpp

@@ -69,3 +69,7 @@ bool BedrockPlugin::preventAttach() {
 void BedrockPlugin::timerFired(SStopwatch* timer) {}
 
 void BedrockPlugin::upgradeDatabase(SQLite& db) {}
+
+bool BedrockPlugin::shouldLockCommitPageOnTableConflict(const string& tableName) const {
+    return true;
+}

BedrockPlugin.h

@@ -78,6 +78,9 @@ class BedrockPlugin {
     // Called when the sync thread is finishing, before destroying DB handles.
     virtual void serverStopping() {}
 
+    // Should a conflict on the given tableName result in locking the associated database page when we try to commit again?
+    virtual bool shouldLockCommitPageOnTableConflict(const string& tableName) const;
+
     // Map of plugin names to functions that will return a new plugin of the given type.
     static map<string, function<BedrockPlugin*(BedrockServer&)>> g_registeredPluginList;
 

BedrockServer.cpp

@@ -801,6 +801,14 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
         // We just spin until the node looks ready to go. Typically, this doesn't happen expect briefly at startup.
         size_t waitCount = 0;
         while (_upgradeInProgress || (getState() != SQLiteNodeState::LEADING && getState() != SQLiteNodeState::FOLLOWING)) {
+
+            // It's feasible that our command times out in this loop. In this case, we do not have a DB object to pass.
+            // The only implication of this is the response does not get the commitCount attached to it.
+            if (BedrockCore::isTimedOut(command, nullptr, this)) {
+                _reply(command);
+                return;
+            }
+
             // This sleep call is pretty ugly, but it should almost never happen. We're accepting the potential
             // looping sleep call for the general case where we just check some bools and continue, instead of
             // avoiding the sleep call but having every thread lock a mutex here on every loop.
@@ -880,7 +888,7 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
             // to be returned to the main queue, where they would have timed out in `peek`, but it was never called
             // because the commands already had a HTTPS request attached, and then they were immediately re-sent to the
             // sync queue, because of the QUORUM consistency requirement, resulting in an endless loop.
-            if (core.isTimedOut(command)) {
+            if (core.isTimedOut(command, &db, this)) {
                 _reply(command);
                 return;
             }
@@ -1030,8 +1038,6 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
                             // loop and send it to followers. NOTE: we don't check for null here, that should be
                             // impossible inside a worker thread.
                             _syncNode->notifyCommit();
-                            SINFO("Committed leader transaction #" << transactionID << "(" << transactionHash << "). Command: '" << command->request.methodLine << "', blocking: "
-                                  << (isBlocking ? "true" : "false"));
                             _conflictManager.recordTables(command->request.methodLine, db.getTablesUsed());
                             // So we must still be leading, and at this point our commit has succeeded, let's
                             // mark it as complete. We add the currentCommit count here as well.
@@ -1043,9 +1049,10 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _command, bool isBlo
                                 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
+                                // the previous commit, the chances are very low 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")) {
+                                // Plugins may define other tables on which we should not lock our next commit.
+                                if (!SStartsWith(lastConflictTable, "journal") && (command->getPlugin() == nullptr || command->getPlugin()->shouldLockCommitPageOnTableConflict(lastConflictTable))) {
                                     lastConflictPage = db.getLastConflictPage();
                                 }
                             }
@@ -1164,6 +1171,12 @@ bool BedrockServer::_wouldCrash(const unique_ptr<BedrockCommand>& command) {
         return false;
     }
 
+    // If this command crashed with more than one set of identifying values, it means
+    // we've already crashed more than one node. Let's fully block this command in that case.
+    if (commandIt->second.size() > 1) {
+        return true;
+    }
+
     // Look at each crash-inducing command that has the same methodLine.
     for (const STable& values : commandIt->second) {
 

Makefile

@@ -15,7 +15,7 @@ INCLUDE = -I$(PROJECT) -I$(PROJECT)/mbedtls/include
 CXXFLAGS = -g -std=c++20 -fPIC -DSQLITE_ENABLE_NORMALIZE $(BEDROCK_OPTIM_COMPILE_FLAG) -Wall -Werror -Wformat-security -Wno-unqualified-std-cast-call -Wno-error=deprecated-declarations $(INCLUDE)
 
 # Amalgamation flags
-AMALGAMATION_FLAGS = -Wno-unused-but-set-variable -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_STAT4 -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_SESSION -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT -DSQLITE_ENABLE_NOOP_UPDATE -DSQLITE_MUTEX_ALERT_MILLISECONDS=20 -DHAVE_USLEEP=1 -DSQLITE_MAX_MMAP_SIZE=17592186044416ull -DSQLITE_SHARED_MAPPING -DSQLITE_ENABLE_NORMALIZE -DSQLITE_MAX_PAGE_COUNT=4294967294 -DSQLITE_DISABLE_PAGECACHE_OVERFLOW_STATS -DSQLITE_DEFAULT_CACHE_SIZE=-51200 -DSQLITE_WAL_BIGHASH
+AMALGAMATION_FLAGS = -Wno-unused-but-set-variable -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_STAT4 -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_SESSION -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT -DSQLITE_ENABLE_NOOP_UPDATE -DSQLITE_MUTEX_ALERT_MILLISECONDS=20 -DHAVE_USLEEP=1 -DSQLITE_MAX_MMAP_SIZE=17592186044416ull -DSQLITE_SHARED_MAPPING -DSQLITE_ENABLE_NORMALIZE -DSQLITE_MAX_PAGE_COUNT=4294967294 -DSQLITE_DISABLE_PAGECACHE_OVERFLOW_STATS -DSQLITE_DEFAULT_CACHE_SIZE=-51200 -DSQLITE_MAX_FUNCTION_ARG=32767 -DSQLITE_WAL_BIGHASH
 
 # All our intermediate, dependency, object, etc files get hidden in here.
 INTERMEDIATEDIR = .build

libstuff/libstuff.cpp

@@ -2722,7 +2722,6 @@ int SQuery(sqlite3* db, const char* e, const string& sql, SQResult& result, int6
 
     // But we log for commit conflicts as well, to keep track of how often this happens with this experimental feature.
     if (extErr == SQLITE_BUSY_SNAPSHOT) {
-        SHMMM("[concurrent] commit conflict.");
         return extErr;
     }
     return error;

sqlitecluster/SQLite.cpp

@@ -393,7 +393,7 @@ bool SQLite::beginTransaction(TRANSACTION_TYPE type) {
     // We actively track transaction counts incrementing and decrementing to log the number of active open transactions at any given moment.
     _sharedData.openTransactionCount++;
 
-    SINFO("[concurrent] Beginning transaction - open transaction count: " << (_sharedData.openTransactionCount));
+    SINFO("Beginning transaction - open transaction count: " << (_sharedData.openTransactionCount));
     uint64_t before = STimeNow();
     _insideTransaction = !SQuery(_db, "starting db transaction", "BEGIN CONCURRENT");
 
@@ -663,7 +663,7 @@ bool SQLite::prepare(uint64_t* transactionID, string* transactionhash) {
     SQResult journalLookupResult;
     SASSERT(!SQuery(_db, "getting commit min", "SELECT MIN(id) FROM " + _journalName, journalLookupResult));
     uint64_t minJournalEntry = journalLookupResult.size() ? SToUInt64(journalLookupResult[0][0]) : 0;
-    
+
     // Note that this can change before we hold the lock on _sharedData.commitLock, but it doesn't matter yet, as we're only
     // using it to truncate the journal. We'll reset this value once we acquire that lock.
     uint64_t commitCount = _sharedData.commitCount;
@@ -677,13 +677,9 @@ bool SQLite::prepare(uint64_t* transactionID, string* transactionhash) {
     // where this journal in particular has accumulated a large backlog.
     static const size_t deleteLimit = 10;
     if (minJournalEntry < oldestCommitToKeep) {
-        auto startUS = STimeNow();
         shared_lock<shared_mutex> lock(_sharedData.writeLock);
         string query = "DELETE FROM " + _journalName + " WHERE id < " + SQ(oldestCommitToKeep) + " LIMIT " + SQ(deleteLimit);
         SASSERT(!SQuery(_db, "Deleting oldest journal rows", query));
-        size_t deletedCount = sqlite3_changes(_db);
-        SINFO("Removed " << deletedCount << " rows from journal " << _journalName << ", oldestToKeep: " << oldestCommitToKeep << ", count:"
-               << commitCount << ", limit: " << _maxJournalSize << ", in " << (STimeNow() - startUS) << "us.");
     }
 
     // We lock this here, so that we can guarantee the order in which commits show up in the database.
@@ -771,9 +767,6 @@ int SQLite::commit(const string& description, function<void()>* preCheckpointCal
     result = SQuery(_db, "committing db transaction", "COMMIT");
     _lastConflictPage = _conflictPage;
     _lastConflictTable = _conflictTable;
-    if (_lastConflictPage) {
-        SINFO(format("part of last conflict page: {}, conflict table: {}",  _conflictPage, _conflictTable));
-    }
 
     // If there were conflicting commits, will return SQLITE_BUSY_SNAPSHOT
     SASSERT(result == SQLITE_OK || result == SQLITE_BUSY_SNAPSHOT);
@@ -834,7 +827,7 @@ int SQLite::commit(const string& description, function<void()>* preCheckpointCal
         _lastConflictPage = 0;
         _lastConflictTable = "";
     } else {
-        SINFO("Commit failed, waiting for rollback.");
+        // The commit failed, we will rollback.
     }
 
     // if we got SQLITE_BUSY_SNAPSHOT, then we're *still* holding commitLock, and it will need to be unlocked by
@@ -883,9 +876,6 @@ void SQLite::rollback() {
         // Finally done with this.
         _insideTransaction = false;
         _uncommittedHash.clear();
-        if (_uncommittedQuery.size()) {
-            SINFO("Rollback successful.");
-        }
         _uncommittedQuery.clear();
 
         // Only unlock the mutex if we've previously locked it. We can call `rollback` to cancel a transaction without

sqlitecluster/SQLiteCore.h

@@ -2,6 +2,11 @@
 class SQLite;
 class SQLiteNode;
 
+#include <cstdint>
+#include <string>
+
+using namespace std;
+
 class SQLiteCore {
   public:
     // Constructor that stores the database object we'll be working on.