diff --git a/src/ifmap/client/ifmap_channel.cc b/src/ifmap/client/ifmap_channel.cc index 3ad25126029..2d8b4fa06a3 100644 --- a/src/ifmap/client/ifmap_channel.cc +++ b/src/ifmap/client/ifmap_channel.cc @@ -129,6 +129,8 @@ IFMapChannel::IFMapChannel(IFMapManager *manager, const std::string& user, response_state_(NONE), sequence_number_(0), recv_msg_cnt_(0), sent_msg_cnt_(0), reconnect_attempts_(0), connection_status_(NOCONN), connection_status_change_at_(UTCTimestampUsec()), + stale_entries_cleanup_timer_(TimerManager::CreateTimer( + *(manager->io_service()), "Stale entries cleanup timer")), end_of_rib_timer_(TimerManager::CreateTimer(*(manager->io_service()), "End of rib timer")) { @@ -144,6 +146,11 @@ IFMapChannel::IFMapChannel(IFMapManager *manager, const std::string& user, b64_auth_str_ = base64_encode(auth_str); } +IFMapChannel::~IFMapChannel() { + TimerManager::DeleteTimer(stale_entries_cleanup_timer_); + TimerManager::DeleteTimer(end_of_rib_timer_); +} + void IFMapChannel::set_connection_status(ConnectionStatus status) { if (connection_status_ != status) { connection_status_ = status; @@ -238,11 +245,18 @@ void IFMapChannel::ReconnectPreparationInMainThr() { void IFMapChannel::ReconnectPreparation() { CHECK_CONCURRENCY("ifmap::StateMachine"); + // The stale entries cleanup timer could be running if connection was reset // in the near past. Stop the timer since we dont want to clean our database // in this case. set_start_stale_entries_cleanup(false); - manager_->ifmap_server()->StopStaleEntriesCleanup(); + StopStaleEntriesCleanupTimer(); + + // The end of rib timer could be running if the initial connection was + // reset in the near past. Stop the timer since we dont want to prematurely + // declare that end of rib has been computed. + StopEndOfRibTimer(); + io_strand_.post( boost::bind(&IFMapChannel::ReconnectPreparationInMainThr, this)); } @@ -606,13 +620,12 @@ int IFMapChannel::ReadPollResponse() { if (start_stale_entries_cleanup()) { // If this is a reconnection, keep re-arming the stale entries // cleanup timer as long as we keep receiving SearchResults. - manager_->ifmap_server()->StartStaleEntriesCleanup(); - } else { - // When the daemon is coming up, as long as we are receiving - // SearchResults, we have not received the entire db. - if (!end_of_rib_computed()) { - StartEndOfRibTimer(); - } + StartStaleEntriesCleanupTimer(); + } + // When the daemon is coming up, as long as we are receiving + // SearchResults, we have not received the entire db. + if (!end_of_rib_computed()) { + StartEndOfRibTimer(); } } string poll_string = reply_str.substr(pos); @@ -636,24 +649,64 @@ int IFMapChannel::ReadPollResponse() { return 0; } -bool IFMapChannel::EndOfRibProcTimeout() { - int timeout = kEndOfRibTimeout; +void IFMapChannel::StartStaleEntriesCleanupTimer() { + CHECK_CONCURRENCY("ifmap::StateMachine"); + if (stale_entries_cleanup_timer_->running()) { + stale_entries_cleanup_timer_->Cancel(); + } + stale_entries_cleanup_timer_->Start(kStaleEntriesCleanupTimeout, + boost::bind(&IFMapChannel::ProcessStaleEntriesTimeout, this), NULL); +} + +void IFMapChannel::StopStaleEntriesCleanupTimer() { + CHECK_CONCURRENCY("ifmap::StateMachine"); + if (stale_entries_cleanup_timer_->running()) { + stale_entries_cleanup_timer_->Cancel(); + } +} + +// Called in the context of the main thread. +bool IFMapChannel::ProcessStaleEntriesTimeout() { + int timeout = kStaleEntriesCleanupTimeout; IFMAP_PEER_DEBUG(IFMapServerConnection, integerToString(timeout), - "millisecond end of rib timer fired"); - set_end_of_rib_computed(true); - process::ConnectionState::GetInstance()->Update(); - return false; + "millisecond stale cleanup timer fired"); + set_start_stale_entries_cleanup(false); + return manager_->ifmap_server()->ProcessStaleEntriesTimeout(); +} + +bool IFMapChannel::StaleEntriesCleanupTimerRunning() { + CHECK_CONCURRENCY("ifmap::StateMachine"); + return stale_entries_cleanup_timer_->running(); } void IFMapChannel::StartEndOfRibTimer() { + CHECK_CONCURRENCY("ifmap::StateMachine"); if (end_of_rib_timer_->running()) { end_of_rib_timer_->Cancel(); } end_of_rib_timer_->Start(kEndOfRibTimeout, - boost::bind(&IFMapChannel::EndOfRibProcTimeout, this), NULL); + boost::bind(&IFMapChannel::ProcessEndOfRibTimeout, this), NULL); +} + +void IFMapChannel::StopEndOfRibTimer() { + CHECK_CONCURRENCY("ifmap::StateMachine"); + if (end_of_rib_timer_->running()) { + end_of_rib_timer_->Cancel(); + } +} + +// Called in the context of the main thread. +bool IFMapChannel::ProcessEndOfRibTimeout() { + int timeout = kEndOfRibTimeout; + IFMAP_PEER_DEBUG(IFMapServerConnection, integerToString(timeout), + "millisecond end of rib timer fired"); + set_end_of_rib_computed(true); + process::ConnectionState::GetInstance()->Update(); + return false; } bool IFMapChannel::EndOfRibTimerRunning() { + CHECK_CONCURRENCY("ifmap::StateMachine"); return end_of_rib_timer_->running(); } @@ -913,7 +966,7 @@ static bool IFMapServerInfoHandleRequest(const Sandesh *sr, server_conn_info.set_start_stale_entries_cleanup( channel->start_stale_entries_cleanup()); server_conn_info.set_stale_entries_cleanup_timer_running( - sctx->ifmap_server()->StaleEntriesCleanupTimerRunning()); + channel->StaleEntriesCleanupTimerRunning()); server_stats.set_rx_msgs(channel->get_recv_msg_cnt()); server_stats.set_tx_msgs(channel->get_sent_msg_cnt()); diff --git a/src/ifmap/client/ifmap_channel.h b/src/ifmap/client/ifmap_channel.h index 704bdc9bf91..3e6308bbf90 100644 --- a/src/ifmap/client/ifmap_channel.h +++ b/src/ifmap/client/ifmap_channel.h @@ -48,7 +48,7 @@ class IFMapChannel { IFMapChannel(IFMapManager *manager, const std::string& user, const std::string& passwd, const std::string& certstore); - virtual ~IFMapChannel() { } + virtual ~IFMapChannel(); void set_sm(IFMapStateMachine *state_machine) { state_machine_ = state_machine; @@ -169,6 +169,7 @@ class IFMapChannel { } PeerTimedoutInfo GetTimedoutInfo(const std::string &host, const std::string &port); + bool StaleEntriesCleanupTimerRunning(); void set_start_stale_entries_cleanup(bool value) { start_stale_entries_cleanup_ = value; } @@ -187,6 +188,8 @@ class IFMapChannel { static const int kSessionKeepaliveInterval = 3; // in seconds static const int kSessionKeepaliveProbes = 5; // count static const int kSessionTcpUserTimeout = 45000; // in milliseconds + + static const int kStaleEntriesCleanupTimeout = 10000; // milliseconds static const int kEndOfRibTimeout = 10000; // milliseconds enum ResponseState { @@ -224,8 +227,12 @@ class IFMapChannel { void SendPollRequestInMainThr(std::string poll_msg); void PollResponseWaitInMainThr(); void ProcResponseInMainThr(size_t bytes_to_read); - bool EndOfRibProcTimeout(); + void StartStaleEntriesCleanupTimer(); + void StopStaleEntriesCleanupTimer(); + bool ProcessStaleEntriesTimeout(); void StartEndOfRibTimer(); + void StopEndOfRibTimer(); + bool ProcessEndOfRibTimeout(); IFMapManager *manager_; boost::asio::ip::tcp::resolver resolver_; @@ -253,6 +260,7 @@ class IFMapChannel { boost::asio::ip::tcp::endpoint endpoint_; TimedoutMap timedout_map_; tbb::atomic start_stale_entries_cleanup_; + Timer *stale_entries_cleanup_timer_; Timer *end_of_rib_timer_; tbb::atomic end_of_rib_computed_; diff --git a/src/ifmap/client/ifmap_manager.cc b/src/ifmap/client/ifmap_manager.cc index ba2cc46635f..63c58c515a4 100644 --- a/src/ifmap/client/ifmap_manager.cc +++ b/src/ifmap/client/ifmap_manager.cc @@ -58,10 +58,6 @@ bool IFMapManager::GetEndOfRibComputed() const { return channel_->end_of_rib_computed(); } -void IFMapManager::SetStartStaleEntriesCleanup(bool value) { - channel_->set_start_stale_entries_cleanup(value); -} - void IFMapManager::GetPeerServerInfo(IFMapPeerServerInfoUI &server_info) { server_info.set_url(get_host_port()); server_info.set_connection_status(channel_->get_connection_status()); diff --git a/src/ifmap/client/ifmap_manager.h b/src/ifmap/client/ifmap_manager.h index f8ef5a7da3c..88da039ac16 100644 --- a/src/ifmap/client/ifmap_manager.h +++ b/src/ifmap/client/ifmap_manager.h @@ -54,7 +54,6 @@ class IFMapManager { virtual void ResetConnection(const std::string &host, const std::string &port); bool PeerDown(); - void SetStartStaleEntriesCleanup(bool value); private: diff --git a/src/ifmap/ifmap_server.cc b/src/ifmap/ifmap_server.cc index 09077008532..f47ff0872f5 100644 --- a/src/ifmap/ifmap_server.cc +++ b/src/ifmap/ifmap_server.cc @@ -191,8 +191,6 @@ IFMapServer::IFMapServer(DB *db, DBGraph *graph, work_queue_(TaskScheduler::GetInstance()->GetTaskId("db::DBTable"), 0, boost::bind(&IFMapServer::ClientWorker, this, _1)), io_service_(io_service), - stale_entries_cleanup_timer_(TimerManager::CreateTimer(*(io_service_), - "Stale entries cleanup timer")), ifmap_manager_(NULL), ifmap_channel_manager_(NULL) { } @@ -205,7 +203,6 @@ void IFMapServer::Initialize() { } void IFMapServer::Shutdown() { - TimerManager::DeleteTimer(stale_entries_cleanup_timer_); vm_uuid_mapper_->Shutdown(); exporter_->Shutdown(); } @@ -373,40 +370,14 @@ bool IFMapServer::ClientNameToIndex(const std::string &id, int *index) { return false; } -bool IFMapServer::StaleEntriesProcTimeout() { - int timeout = kStaleEntriesCleanupTimeout; - IFMAP_DEBUG(IFMapStaleEntriesCleanupTimerFired, integerToString(timeout), - "millisecond stale entries cleanup timer fired"); - SetStartStaleEntriesCleanup(false); +bool IFMapServer::ProcessStaleEntriesTimeout() { IFMapStaleEntriesCleaner *cleaner = new IFMapStaleEntriesCleaner(db_, graph_, this); - TaskScheduler *scheduler = TaskScheduler::GetInstance(); scheduler->Enqueue(cleaner); return false; } -void IFMapServer::StartStaleEntriesCleanup() { - CHECK_CONCURRENCY("ifmap::StateMachine"); - if (stale_entries_cleanup_timer_->running()) { - stale_entries_cleanup_timer_->Cancel(); - } - stale_entries_cleanup_timer_->Start(kStaleEntriesCleanupTimeout, - boost::bind(&IFMapServer::StaleEntriesProcTimeout, this), NULL); -} - -bool IFMapServer::StaleEntriesCleanupTimerRunning() { - CHECK_CONCURRENCY("ifmap::StateMachine"); - return stale_entries_cleanup_timer_->running(); -} - -void IFMapServer::StopStaleEntriesCleanup() { - CHECK_CONCURRENCY("ifmap::StateMachine"); - if (stale_entries_cleanup_timer_->running()) { - stale_entries_cleanup_timer_->Cancel(); - } -} - void IFMapServer::ProcessVmSubscribe(std::string vr_name, std::string vm_uuid, bool subscribe, bool has_vms) { IFMapVmSubscribe *vm_sub = new IFMapVmSubscribe(db_, graph_, this, diff --git a/src/ifmap/ifmap_server.h b/src/ifmap/ifmap_server.h index 765f5daf2cb..efdc472b3ab 100644 --- a/src/ifmap/ifmap_server.h +++ b/src/ifmap/ifmap_server.h @@ -74,11 +74,6 @@ class IFMapServer { virtual uint64_t get_ifmap_channel_sequence_number() { return ifmap_manager_->GetChannelSequenceNumber(); } - void SetStartStaleEntriesCleanup(bool value) { - if (ifmap_manager_) { - ifmap_manager_->SetStartStaleEntriesCleanup(value); - } - } void set_ifmap_channel_manager(IFMapChannelManager *manager) { ifmap_channel_manager_ = manager; } @@ -104,13 +99,9 @@ class IFMapServer { const CmSz_t GetIndexMapSize() const { return index_map_.size(); } void GetUIInfo(IFMapServerInfoUI *server_info); bool ClientNameToIndex(const std::string &id, int *index); - void StartStaleEntriesCleanup(); - void StopStaleEntriesCleanup(); - bool StaleEntriesCleanupTimerRunning(); + bool ProcessStaleEntriesTimeout(); private: - static const int kStaleEntriesCleanupTimeout = 10000; // milliseconds - friend class IFMapServerTest; friend class IFMapRestartTest; friend class ShowIFMapXmppClientInfo; @@ -131,7 +122,6 @@ class IFMapServer { void CleanupUuidMapper(IFMapClient *client); void ClientExporterSetup(IFMapClient *client); void ClientExporterCleanup(IFMapClient *client); - bool StaleEntriesProcTimeout(); const ClientMap &GetClientMap() const { return client_map_; } void SimulateDeleteClient(IFMapClient *client); @@ -146,7 +136,6 @@ class IFMapServer { IndexMap index_map_; WorkQueue work_queue_; boost::asio::io_service *io_service_; - Timer *stale_entries_cleanup_timer_; IFMapManager *ifmap_manager_; IFMapChannelManager *ifmap_channel_manager_; }; diff --git a/src/ifmap/test/ifmap_restart_test.cc b/src/ifmap/test/ifmap_restart_test.cc index bea7b4d74aa..8192f214ee9 100644 --- a/src/ifmap/test/ifmap_restart_test.cc +++ b/src/ifmap/test/ifmap_restart_test.cc @@ -134,8 +134,8 @@ class IFMapRestartTest : public ::testing::Test { return tbl->FindNode(name); } - void StaleNodesProcTimeout() { - server_.StaleEntriesProcTimeout(); + void ProcessStaleEntriesTimeout() { + server_.ProcessStaleEntriesTimeout(); } DB db_; @@ -206,7 +206,7 @@ TEST_F(IFMapRestartTest, BasicTest) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // The nodes had seq-num 2 and all of them should still exist after cleanup @@ -219,7 +219,7 @@ TEST_F(IFMapRestartTest, BasicTest) { // Update the channel's seq-num to 3 and trigger cleanup server_.set_ifmap_channel_sequence_number(3); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // Nodes had seq-num 2 and all of them should be gone @@ -281,7 +281,7 @@ TEST_F(IFMapRestartTest, BasicTest1) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // The nodes with seq-num 1 should be gone i.e. virtual-network blue @@ -343,7 +343,7 @@ TEST_F(IFMapRestartTest, PropertiesTest) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); idn1 = TableLookup("domain", "user1"); @@ -419,7 +419,7 @@ TEST_F(IFMapRestartTest, LinkAttr) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // Nodes had seq-num 2 and all of them should be gone @@ -522,7 +522,7 @@ TEST_F(IFMapRestartTest, LinkAttrWithProperties) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // Sequence numbers match. Everything should exist. @@ -541,7 +541,7 @@ TEST_F(IFMapRestartTest, LinkAttrWithProperties) { // Update the channel's seq-num to 3 and trigger cleanup server_.set_ifmap_channel_sequence_number(3); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // Only the vn should exist. We should not find the other nodes. @@ -556,7 +556,7 @@ TEST_F(IFMapRestartTest, LinkAttrWithProperties) { // Update the channel's seq-num to 4 and trigger cleanup server_.set_ifmap_channel_sequence_number(4); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // The vn should also be cleaned up. @@ -655,7 +655,7 @@ TEST_F(IFMapRestartTest, MultipleAttrChangesWithSeqNumChange) { // Update the channel's seq-num to 2 and trigger cleanup server_.set_ifmap_channel_sequence_number(2); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // Sequence numbers match. Everything should exist. @@ -676,7 +676,7 @@ TEST_F(IFMapRestartTest, MultipleAttrChangesWithSeqNumChange) { // Update the channel's seq-num to 3 and trigger cleanup server_.set_ifmap_channel_sequence_number(3); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // All the nodes should exist @@ -693,7 +693,7 @@ TEST_F(IFMapRestartTest, MultipleAttrChangesWithSeqNumChange) { // Update the channel's seq-num to 4 and trigger cleanup server_.set_ifmap_channel_sequence_number(4); - StaleNodesProcTimeout(); + ProcessStaleEntriesTimeout(); task_util::WaitForIdle(); // All the nodes should be cleaned up