diff --git a/sqlitecluster/SQLiteNode.cpp b/sqlitecluster/SQLiteNode.cpp index c49499d61..6cc77cf94 100644 --- a/sqlitecluster/SQLiteNode.cpp +++ b/sqlitecluster/SQLiteNode.cpp @@ -2059,12 +2059,16 @@ void SQLiteNode::_changeState(SQLiteNodeState newState, uint64_t commitIDToCance } } - // If we're switching from LEADING or STANDINGDOWN to anything else (aside from the case where we switch from LEADING to STANDINGDOWN), we unblock commits. - if ((_state == SQLiteNodeState::LEADING || _state == SQLiteNodeState::STANDINGDOWN) && newState != SQLiteNodeState::STANDINGDOWN) { - if (_commitsBlocked) { - _commitsBlocked = false; - _db.exclusiveUnlockDB(); - } + // If we've blocked commits, unblock before switching states. This implies we *were* leading and now are not, + // so commits remaining blocked doesn't really make sense any more anyway, except in the case where we're switching + // from LEADING to STANDINGDOWN in which case we *could* keep this blocked, though that'd be weird, too. We'd + // need to wait around with commits blocked until the cluster caught up, so that we could really start shutting down, which + // stops processing new commands anyway. We might as well just run through whatever's waiting. + // But also, there's another reason to do this even in the LEADING->STANDINGDOWN case, and that's because the locks acquired in + // exclusiveLockDB() are not recursive, so we need to release them before we call `exclusiveLockDB` again just after this `if` block. + if (_commitsBlocked) { + _commitsBlocked = false; + _db.exclusiveUnlockDB(); } // IMPORTANT: Don't return early or throw from this method after here.