Merge pull request #2053 from Expensify/flo_onyxTables

Don't page lock on onyx tables

Author: tylerkaraszewski

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;

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

@@ -1049,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();
                                 }
                             }