Merge pull request #2083 from Expensify/main

Update expensify_prod branch

sqlitecluster/SQLite.cpp

@@ -508,7 +508,7 @@ bool SQLite::read(const string& query, SQResult& result, bool skipInfoWarn) cons
     } else {
         _isDeterministicQuery = true;
         queryResult = !SQuery(_db, "read only query", query, result, 2000 * STIME_US_PER_MS, skipInfoWarn);
-        if (_isDeterministicQuery && queryResult) {
+        if (_isDeterministicQuery && queryResult && insideTransaction()) {
             _queryCache.emplace(make_pair(query, result));
         }
     }

sqlitecluster/SQLite.h

@@ -216,7 +216,7 @@ class SQLite {
     const string getMostRecentSQLiteErrorLog() const;
 
     // Returns true if we're inside an uncommitted transaction.
-    bool insideTransaction() { return _insideTransaction; }
+    bool insideTransaction() const { return _insideTransaction; }
 
     // Looks up the exact SQL of a paricular commit to the database, as well as gets the SHA1 hash of the database
     // immediately following tha commit.

sqlitecluster/SQLiteNode.cpp

@@ -1,4 +1,5 @@
 #include "SQLiteNode.h"
+#include "sqlitecluster/SQLite.h"
 
 #include <unistd.h>
 
@@ -593,6 +594,11 @@ bool SQLiteNode::update() {
     ///     period and go SEARCHING.
     ///
     case SQLiteNodeState::SYNCHRONIZING: {
+        if (_isShuttingDown) {
+            _changeState(SQLiteNodeState::SEARCHING);
+            return false;
+        }
+
         SASSERTWARN(_syncPeer);
         SASSERTWARN(!_leadPeer);
         SASSERTWARN(_db.getUncommittedHash().empty());
@@ -631,10 +637,15 @@ bool SQLiteNode::update() {
         SASSERTWARN(_db.getUncommittedHash().empty());
 
         if (_isShuttingDown) {
-            SWARN("Shutting down in WAITING, how did we get here?");
+            // This can happen because of fast stand down.
+            // If there are commands in `peek` when we start stand down, we will need to escalate those after stand down to the new leader.
+            // So the sync node starts the process of going through:
+            // LEADING -> STANDINGDOWN -> SEARCHING -> SYNCHRONIZING -> WAITING -> FOLLLOWING -> SEARCHING
+            // However, if we complete the first part of this and get through STANDINGDOWN and there are *no* commands
+            // left to handle, the node could be in any of SEARCHING, SYNCHRONIZING, WAITING, or FOLLOWING.
+            // We switch to SEARCHING just for consistency, and exit.
+            _changeState(SQLiteNodeState::SEARCHING);
 
-            // Return false to allow the poll loop to run again. This is here for legacy reasons form before the above warning was added.
-            // Probably this whole block can be removed if that warning never fires.
             return false;
         }
 
@@ -1095,6 +1106,10 @@ bool SQLiteNode::update() {
     ///     timeout, go SEARCHING.
     ///
     case SQLiteNodeState::SUBSCRIBING:
+        if (_isShuttingDown) {
+            _changeState(SQLiteNodeState::SEARCHING);
+            return false;
+        }
         SASSERTWARN(!_syncPeer);
         SASSERTWARN(_leadPeer);
         SASSERTWARN(_db.getUncommittedHash().empty());