Skip to content

Commit

Permalink
Clearing service instance DB State after STOP command
Browse files Browse the repository at this point in the history
If Delete of service instance is received before it was marked
unusable, to stop the namespace, StopServiceInstance command is issued,
which invokes the deletion of the namespace script. And immediately after
this, DB state is cleared. Clearing of DB State can result in deleting
of service instance DBEntry before namespace script completion. SIGCHILD
event after script completion looks for service instance that triggered the
task, and results in crash as the service isntance is already deleted.
To avoid crash, the DBState is cleared after processing the SIGCHILD
event. Also corresponding to the service instance, there can be many
tasks queued for execution. To identify how any SIGCHILDS to wait for,
running tasks cound is maintained in DBState. This could is manipulated
everytime the task is created, destroyed, error handled corresponding to
the service instance.
Also, the config filters for missing tables, which treat the NULL uuid as node
deletes is also added

Change-Id: I406f9e3d70768151e4390955153bc3176d3da99f
closes-bug: #1453956
  • Loading branch information
divakardhar committed Jun 1, 2015
1 parent 71780ac commit 06b38bc
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 13 deletions.
19 changes: 19 additions & 0 deletions src/vnsw/agent/cfg/cfg_filter.cc
Expand Up @@ -81,9 +81,28 @@ void CfgFilter::Init() {
agent_cfg_->cfg_acl_table()->RegisterPreFilter
(boost::bind(&CfgFilter::CheckProperty, this, _1, _2, _3,
AccessControlList::ID_PERMS));

agent_cfg_->cfg_loadbalancer_table()->RegisterPreFilter
(boost::bind(&CfgFilter::CheckProperty, this, _1, _2, _3,
LoadbalancerPool::ID_PERMS));

agent_cfg_->cfg_service_instance_table()->RegisterPreFilter
(boost::bind(&CfgFilter::CheckProperty, this, _1, _2, _3,
ServiceInstance::ID_PERMS));

agent_cfg_->cfg_security_group_table()->RegisterPreFilter
(boost::bind(&CfgFilter::CheckProperty, this, _1, _2, _3,
SecurityGroup::ID_PERMS));

}

void CfgFilter::Shutdown() {
agent_cfg_->cfg_vm_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_vn_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_vm_interface_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_acl_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_loadbalancer_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_service_instance_table()->RegisterPreFilter(NULL);
agent_cfg_->cfg_security_group_table()->RegisterPreFilter(NULL);

}
20 changes: 18 additions & 2 deletions src/vnsw/agent/cfg/cfg_init.cc
Expand Up @@ -123,9 +123,9 @@ void AgentConfig::RegisterDBClients(DB *db) {
cfg_listener_->Register("virtual-network-network-ipam",
boost::bind(&VnTable::IpamVnSync, _1), -1);
cfg_listener_->Register("network-ipam", boost::bind(&DomainConfig::IpamSync,
agent_->domain_config_table(), _1), NetworkIpam::ID_PERMS);
agent_->domain_config_table(), _1), -1);
cfg_listener_->Register("virtual-DNS", boost::bind(&DomainConfig::VDnsSync,
agent_->domain_config_table(), _1), VirtualDns::ID_PERMS);
agent_->domain_config_table(), _1), -1);
cfg_listener_->Register
("virtual-machine-interface-routing-instance",
boost::bind(&InterfaceTable::VmInterfaceVrfSync,
Expand Down Expand Up @@ -203,6 +203,22 @@ void AgentConfig::RegisterDBClients(DB *db) {
"interface-route-table")));
assert(cfg_route_table_);

cfg_loadbalancer_table_ = (static_cast<IFMapAgentTable *>
(IFMapTable::FindTable(agent_->db(),
"loadbalancer_pool")));
assert(cfg_loadbalancer_table_);

cfg_service_instance_table_ = (static_cast<IFMapAgentTable *>
(IFMapTable::FindTable(agent_->db(),
"service_instance")));
assert(cfg_service_instance_table_);

cfg_security_group_table_ = (static_cast<IFMapAgentTable *>
(IFMapTable::FindTable(agent_->db(),
"security_group")));
assert(cfg_security_group_table_);


cfg_interface_client_->Init();
}

Expand Down
17 changes: 17 additions & 0 deletions src/vnsw/agent/cfg/cfg_init.h
Expand Up @@ -57,6 +57,18 @@ class AgentConfig {
return cfg_service_template_table_;
}

IFMapAgentTable *cfg_loadbalancer_table() const {
return cfg_loadbalancer_table_;
}

IFMapAgentTable *cfg_service_instance_table() const {
return cfg_service_instance_table_;
}

IFMapAgentTable *cfg_security_group_table() const {
return cfg_security_group_table_;
}

Agent *agent() const { return agent_; }
CfgFilter *cfg_filter() const { return cfg_filter_.get(); }
CfgListener *cfg_listener() const { return cfg_listener_.get(); }
Expand Down Expand Up @@ -106,6 +118,11 @@ class AgentConfig {
IFMapAgentTable *cfg_vm_port_vrf_table_;
IFMapAgentTable *cfg_route_table_;
IFMapAgentTable *cfg_service_template_table_;
IFMapAgentTable *cfg_loadbalancer_table_;
IFMapAgentTable *cfg_service_instance_table_;
IFMapAgentTable *cfg_security_group_table_;



DISALLOW_COPY_AND_ASSIGN(AgentConfig);
};
Expand Down
59 changes: 49 additions & 10 deletions src/vnsw/agent/oper/instance_manager.cc
Expand Up @@ -215,13 +215,16 @@ void InstanceManager::SigChldEventHandler(InstanceManagerChildEvent event) {
if (!task_queue->Empty()) {
InstanceTask *task = task_queue->Front();
if (task->pid() == event.pid) {

//Get the sevice instance first, to delete the state later
ServiceInstance* svc_instance = GetSvcInstance(task);
UpdateStateStatusType(task, event.status);

task_queue->Pop();
delete task;

task_queue->StopTimer();

DeleteState(svc_instance);
ScheduleNextTask(task_queue);
return;
}
Expand Down Expand Up @@ -316,6 +319,23 @@ void InstanceManager::ClearState(ServiceInstance *svc_instance) {
svc_instance->ClearState(agent_->service_instance_table(), si_listener_);
}

bool InstanceManager::DeleteState(ServiceInstance *svc_instance) {

if (!svc_instance || !svc_instance->IsDeleted()) {
return false;
}

InstanceState *state = GetState(svc_instance);
if (state && !state->tasks_running()) {
ClearState(svc_instance);
delete state;
ClearLastCmdType(svc_instance);
return true;
}

return false;
}

void InstanceManager::InitSigHandler(AgentSignal *signal) {
signal->RegisterChildHandler(
boost::bind(&InstanceManager::HandleSigChild, this, _1, _2, _3, _4));
Expand All @@ -332,6 +352,7 @@ void InstanceManager::StateClear() {
if (state != NULL) {
entry->ClearState(agent_->service_instance_table(), si_listener_);
delete state;
ClearLastCmdType(static_cast<ServiceInstance *>(entry));
}
next = partition->GetNext(entry);
}
Expand Down Expand Up @@ -420,6 +441,11 @@ void InstanceManager::ScheduleNextTask(InstanceTaskQueue *task_queue) {

task_queue->StopTimer();
task_queue->Pop();
ServiceInstance* svc_instance = GetSvcInstance(task);
if (state && svc_instance)
state->decr_tasks_running();
task_svc_instances_.erase(task);
DeleteState(svc_instance);

delete task;
} else {
Expand All @@ -431,7 +457,7 @@ void InstanceManager::ScheduleNextTask(InstanceTaskQueue *task_queue) {
}

ServiceInstance *InstanceManager::GetSvcInstance(InstanceTask *task) const {
std::map<InstanceTask *, ServiceInstance*>::const_iterator iter =
TaskSvcMap::const_iterator iter =
task_svc_instances_.find(task);
if (iter != task_svc_instances_.end()) {
return iter->second;
Expand All @@ -441,15 +467,23 @@ 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<TaskSvcMap::iterator, bool> 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<InstanceTask *, ServiceInstance*>::iterator iter =
task_svc_instances_.begin();
for (TaskSvcMap::iterator iter = task_svc_instances_.begin();
iter != task_svc_instances_.end(); ++iter) {
if (task == iter->first) {
ServiceInstance *svc_instance = iter->second;
InstanceState *state = GetState(svc_instance);
assert(state);
state->decr_tasks_running();
task_svc_instances_.erase(iter);
return svc_instance;
}
Expand All @@ -459,11 +493,16 @@ ServiceInstance *InstanceManager::UnregisterSvcInstance(InstanceTask *task) {
}

void InstanceManager::UnregisterSvcInstance(ServiceInstance *svc_instance) {
std::map<InstanceTask *, ServiceInstance*>::iterator iter =

InstanceState *state = GetState(svc_instance);
assert(state);

TaskSvcMap::iterator iter =
task_svc_instances_.begin();
while(iter != task_svc_instances_.end()) {
if (svc_instance == iter->second) {
task_svc_instances_.erase(iter++);
state->decr_tasks_running();
} else {
++iter;
}
Expand Down Expand Up @@ -636,14 +675,14 @@ void InstanceManager::EventObserver(

InstanceState *state = GetState(svc_instance);
if (svc_instance->IsDeleted()) {
UnregisterSvcInstance(svc_instance);
if (state) {
if (GetLastCmdType(svc_instance) == Start) {
StopNetNS(svc_instance, state);
SetLastCmdType(svc_instance, Stop);
}

ClearState(svc_instance);
delete state;
if (DeleteState(svc_instance))
return;
}
ClearLastCmdType(svc_instance);
} else {
Expand Down Expand Up @@ -723,7 +762,7 @@ void InstanceManager::SetNamespaceStorePath(std::string path) {
* InstanceState class
*/
InstanceState::InstanceState() : DBState(),
pid_(0), status_(0), status_type_(0) {
pid_(0), status_(0), status_type_(0), tasks_running_(0) {
}

void InstanceState::Clear() {
Expand Down
20 changes: 19 additions & 1 deletion src/vnsw/agent/oper/instance_manager.h
Expand Up @@ -104,6 +104,7 @@ class InstanceManager {
InstanceState *GetState(InstanceTask* task) const;
void SetState(ServiceInstance *svc_instance, InstanceState *state);
void ClearState(ServiceInstance *svc_instance);
bool DeleteState(ServiceInstance *svc_instance);
void UpdateStateStatusType(InstanceTask* task, int status);

void SetLastCmdType(ServiceInstance *svc_instance, int last_cmd_type);
Expand Down Expand Up @@ -138,7 +139,8 @@ class InstanceManager {
WorkQueue<InstanceManagerChildEvent> work_queue_;

std::vector<InstanceTaskQueue *> task_queues_;
std::map<InstanceTask *, ServiceInstance *> task_svc_instances_;
typedef std::map<InstanceTask *, ServiceInstance *> TaskSvcMap;
TaskSvcMap task_svc_instances_;
std::map<std::string, int> last_cmd_types_;
std::string loadbalancer_config_path_;
std::string namespace_store_path_;
Expand Down Expand Up @@ -207,6 +209,21 @@ class InstanceState : public DBState {
return status_type_;
}

int tasks_running() const {
return tasks_running_;
}

int incr_tasks_running() {
return ++tasks_running_;
}

int decr_tasks_running() {
tasks_running_--;
assert(!(tasks_running_ < 0));
return tasks_running_;
}


void Clear();

private:
Expand All @@ -215,6 +232,7 @@ class InstanceState : public DBState {
std::string errors_;
std::string cmd_;
int status_type_;
int tasks_running_;

ServiceInstance::Properties properties_;

Expand Down

0 comments on commit 06b38bc

Please sign in to comment.