From 41dfe4898e793708a610ede128d7a58834b3c40b Mon Sep 17 00:00:00 2001 From: Divakar Date: Tue, 9 Jun 2015 10:10:17 -0700 Subject: [PATCH] Instance manager task queue size 1) To insert the Instnace task into task queue, hash of service instance uuid is used. To identify the numer of task queues avaialble, .capacity() routine is used on vector, whose value mgiht be more than the size of the vector. If task is inserted into a vector position that is more than the size of vector, exception can occur. To avoid this, .size() routine is invoked. 2) While inserting the task into the map of (service instance, task), strict error check is made to ensure that we dont insert the same task again into the map. 3) While starting the task, if there is an error, task is removed from above map without unregistering the service instance, which might lead to wrong refcounting. To avaoid this, unregisterserviceinstane() is invoked which takes care of refcounting. Change-Id: I1608be4f47d3a981aa1f8b7c9f48e0e80d21fc33 partial-bug: #1462042 --- src/vnsw/agent/oper/instance_manager.cc | 16 +++++++++------- src/vnsw/agent/oper/instance_manager.h | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/vnsw/agent/oper/instance_manager.cc b/src/vnsw/agent/oper/instance_manager.cc index fcf45d942fc..cfc470997f9 100644 --- a/src/vnsw/agent/oper/instance_manager.cc +++ b/src/vnsw/agent/oper/instance_manager.cc @@ -409,7 +409,7 @@ void InstanceManager::Enqueue(InstanceTask *task, InstanceTaskQueue *InstanceManager::GetTaskQueue(const std::string &str) { boost::hash hash; - int index = hash(str) % task_queues_.capacity(); + int index = hash(str) % task_queues_.size(); return task_queues_[index]; } @@ -438,7 +438,7 @@ bool InstanceManager::StartTask(InstanceTaskQueue *task_queue, } task_queue->Pop(); - task_svc_instances_.erase(task); + UnregisterSvcInstance(task); delete task; return false; @@ -488,7 +488,7 @@ void InstanceManager::ScheduleNextTask(InstanceTaskQueue *task_queue) { } ServiceInstance *InstanceManager::GetSvcInstance(InstanceTask *task) const { - std::map::const_iterator iter = + TaskSvcMap::const_iterator iter = task_svc_instances_.find(task); if (iter != task_svc_instances_.end()) { return iter->second; @@ -498,14 +498,17 @@ ServiceInstance *InstanceManager::GetSvcInstance(InstanceTask *task) const { void InstanceManager::RegisterSvcInstance(InstanceTask *task, ServiceInstance *svc_instance) { - task_svc_instances_.insert(std::make_pair(task, svc_instance)); + pair result = + task_svc_instances_.insert(std::make_pair(task, svc_instance)); + assert(result.second); + InstanceState *state = GetState(svc_instance); assert(state); state->incr_tasks_running(); } ServiceInstance *InstanceManager::UnregisterSvcInstance(InstanceTask *task) { - for (std::map::iterator iter = + for (TaskSvcMap::iterator iter = task_svc_instances_.begin(); iter != task_svc_instances_.end(); ++iter) { if (task == iter->first) { @@ -526,8 +529,7 @@ void InstanceManager::UnregisterSvcInstance(ServiceInstance *svc_instance) { InstanceState *state = GetState(svc_instance); assert(state); - std::map::iterator iter = - task_svc_instances_.begin(); + TaskSvcMap::iterator iter = task_svc_instances_.begin(); while(iter != task_svc_instances_.end()) { if (svc_instance == iter->second) { task_svc_instances_.erase(iter++); diff --git a/src/vnsw/agent/oper/instance_manager.h b/src/vnsw/agent/oper/instance_manager.h index d587695d449..f1b81c8f268 100644 --- a/src/vnsw/agent/oper/instance_manager.h +++ b/src/vnsw/agent/oper/instance_manager.h @@ -145,7 +145,8 @@ class InstanceManager { WorkQueue work_queue_; std::vector task_queues_; - std::map task_svc_instances_; + typedef std::map TaskSvcMap; + TaskSvcMap task_svc_instances_; std::map last_cmd_types_; std::string loadbalancer_config_path_; std::string namespace_store_path_;