From 02db59233a678ed973845c6ebe3f1bbab4649467 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Mon, 26 Sep 2022 14:54:48 +0000 Subject: [PATCH] Remove StateManager handling of deprecated Initializing state Neither distributors nor content nodes ever report their state as Initializing as part of their startup sequence; they go straight from Down to Up. Remove complicated init progress delta reporting that is no longer needed. --- .../tests/storageserver/statemanagertest.cpp | 6 +-- .../storage/storageserver/statemanager.cpp | 35 ++----------- .../storage/storageserver/statemanager.h | 49 +++++++++---------- 3 files changed, 32 insertions(+), 58 deletions(-) diff --git a/storage/src/tests/storageserver/statemanagertest.cpp b/storage/src/tests/storageserver/statemanagertest.cpp index 729976d2dce1..5a43f04072d9 100644 --- a/storage/src/tests/storageserver/statemanagertest.cpp +++ b/storage/src/tests/storageserver/statemanagertest.cpp @@ -176,9 +176,9 @@ TEST_F(StateManagerTest, reported_node_state) { // Add a state listener to check that we get events. MyStateListener stateListener(*_manager); _manager->addStateListener(stateListener); - // Test that initial state is initializing + // Test that initial state is Down auto nodeState = _manager->getReportedNodeState(); - EXPECT_EQ("s:i b:58 i:0 t:1", nodeState->toString(false)); + EXPECT_EQ("s:d b:58 t:1", nodeState->toString(false)); // Test that it works to update the state { auto lock = _manager->grabStateChangeLock(); @@ -236,7 +236,7 @@ TEST_F(StateManagerTest, reported_node_state) { _manager->setReportedNodeState(ns); } std::string expectedEvents = - "s:i b:58 i:0 t:1 -> s:u b:58 t:1\n" + "s:d b:58 t:1 -> s:u b:58 t:1\n" "s:u b:58 t:1 -> s:s b:58 t:1 m:Stopping\\x20node\n"; EXPECT_EQ(expectedEvents, stateListener.ost.str()); } diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index b1ea000e9bcf..b75400e13083 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -35,7 +35,7 @@ StateManager::StateManager(StorageComponentRegister& compReg, _stateLock(), _stateCond(), _listenerLock(), - _nodeState(std::make_shared(_component.getNodeType(), lib::State::INITIALIZING)), + _nodeState(std::make_shared(_component.getNodeType(), lib::State::DOWN)), _nextNodeState(), _systemState(std::make_shared(lib::ClusterState())), _nextSystemState(), @@ -43,8 +43,6 @@ StateManager::StateManager(StorageComponentRegister& compReg, _stateListeners(), _queuedStateRequests(), _threadLock(), - _lastProgressUpdateCausingSend(0), - _progressLastInitStateSend(-1), _systemStateHistory(), _systemStateHistorySize(50), _hostInfo(std::move(hostInfo)), @@ -238,31 +236,8 @@ StateManager::notifyStateListeners() break; // No change } if (_nextNodeState) { - assert(!(_nodeState->getState() == State::UP - && _nextNodeState->getState() == State::INITIALIZING)); - - if (_nodeState->getState() == State::INITIALIZING - && _nextNodeState->getState() == State::INITIALIZING - && ((_component.getClock().getTimeInMillis() - _lastProgressUpdateCausingSend) - < framework::MilliSecTime(1000)) - && _nextNodeState->getInitProgress() < 1 - && (_nextNodeState->getInitProgress() - _progressLastInitStateSend) < 0.01) - { - // For this special case, where we only have gotten a little - // initialization progress and we have reported recently, - // don't trigger sending get node state reply yet. - } else { - newState = _nextNodeState; - if (!_queuedStateRequests.empty() - && _nextNodeState->getState() == State::INITIALIZING) - { - _lastProgressUpdateCausingSend = _component.getClock().getTimeInMillis(); - _progressLastInitStateSend = newState->getInitProgress(); - } else { - _lastProgressUpdateCausingSend = framework::MilliSecTime(0); - _progressLastInitStateSend = -1; - } - } + assert(_nextNodeState->getState() != State::INITIALIZING); + newState = _nextNodeState; _nodeState = _nextNodeState; _nextNodeState.reset(); } @@ -414,14 +389,14 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd) "%" PRId64 " milliseconds unless a node state change " "happens before that time.", msTimeout, msTimeout * 800 / 1000); - TimeStatePair pair( + TimeStateCmdPair pair( _component.getClock().getTimeInMillis() + framework::MilliSecTime(msTimeout * 800 / 1000), cmd); _queuedStateRequests.emplace_back(std::move(pair)); } else { LOG(debug, "Answered get node state request right away since it " - "thought we were in nodestate %s, while our actual " + "thought we were in node state %s, while our actual " "node state is currently %s and we didn't just reply to " "existing request.", cmd->getExpectedState() == nullptr ? "unknown" diff --git a/storage/src/vespa/storage/storageserver/statemanager.h b/storage/src/vespa/storage/storageserver/statemanager.h index 194991723f47..0df07d048ebb 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.h +++ b/storage/src/vespa/storage/storageserver/statemanager.h @@ -41,36 +41,35 @@ class StateManager : public NodeStateUpdater, private framework::Runnable, private vespalib::JsonStreamTypes { - StorageComponent _component; - metrics::MetricManager& _metricManager; - mutable std::mutex _stateLock; - std::condition_variable _stateCond; - std::mutex _listenerLock; - std::shared_ptr _nodeState; - std::shared_ptr _nextNodeState; using ClusterStateBundle = lib::ClusterStateBundle; + using TimeStateCmdPair = std::pair; + using TimeSysStatePair = std::pair>; + + StorageComponent _component; + metrics::MetricManager& _metricManager; + mutable std::mutex _stateLock; + std::condition_variable _stateCond; + std::mutex _listenerLock; + std::shared_ptr _nodeState; + std::shared_ptr _nextNodeState; std::shared_ptr _systemState; std::shared_ptr _nextSystemState; - uint32_t _reported_host_info_cluster_state_version; - std::list _stateListeners; - typedef std::pair TimeStatePair; - std::list _queuedStateRequests; - mutable std::mutex _threadLock; - std::condition_variable _threadCond; - framework::MilliSecTime _lastProgressUpdateCausingSend; - vespalib::Double _progressLastInitStateSend; - using TimeSysStatePair = std::pair>; - std::deque _systemStateHistory; - uint32_t _systemStateHistorySize; - std::unique_ptr _hostInfo; - framework::Thread::UP _thread; + uint32_t _reported_host_info_cluster_state_version; + std::list _stateListeners; + std::list _queuedStateRequests; + mutable std::mutex _threadLock; + std::condition_variable _threadCond; + std::deque _systemStateHistory; + uint32_t _systemStateHistorySize; + std::unique_ptr _hostInfo; + framework::Thread::UP _thread; // Controllers that have observed a GetNodeState response sent _after_ // immediately_send_get_node_state_replies() has been invoked. - std::unordered_set _controllers_observed_explicit_node_state; - bool _noThreadTestMode; - bool _grabbedExternalLock; - std::atomic _notifyingListeners; - std::atomic _requested_almost_immediate_node_state_replies; + std::unordered_set _controllers_observed_explicit_node_state; + bool _noThreadTestMode; + bool _grabbedExternalLock; + std::atomic _notifyingListeners; + std::atomic _requested_almost_immediate_node_state_replies; public: explicit StateManager(StorageComponentRegister&, metrics::MetricManager&,