Skip to content

Commit

Permalink
Control node crash when handling config client reinit:
Browse files Browse the repository at this point in the history
The config client can be reinit through a signal or if some config
params changed. While processing the reinit, the config manager tries
to shutdown and recreate the config db client. The config client and
its associated partition tasks cannot be running since they are
mutually exclusive tasks to the client init task and this is a valid
assumption. However, it is possible that the tasks have been scheduled
to run but yet to begin execution. If that is the case and we try to
shutdown the config db client/partition, TaskTriggers assert if there
are scheduled tasks. This is not a problem for WorkQueue and Task
classes. Fixing this for TaskTrigger tasks by checking if they have
been scheduled and retrying the config init.
UT will be done at a later time when the test infra for reinit code
is added. For now, verified that the config client tests pass.

Change-Id: Id6d9c6aed32d32688d88932a8ec6c85a5207ad28
Partial-Bug: 1786154
(cherry picked from commit 8ae5f21)
  • Loading branch information
maheshmns committed Sep 13, 2018
1 parent e9096fb commit 39a4814
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 3 deletions.
22 changes: 22 additions & 0 deletions config-client-mgr/config_cassandra_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,24 @@ bool ConfigCassandraClient::BulkDataSync() {
return true;
}

bool ConfigCassandraClient::IsTaskTriggered() const {
// If FQNameReader task has been triggered return true.
if (fq_name_reader_->IsSet()) {
return true;
}

/**
* Walk the partitions and check if ConfigReader task has
* been triggered in any of them. If so, return true.
*/
BOOST_FOREACH(ConfigCassandraPartition *partition, partitions_) {
if (partition->IsTaskTriggered()) {
return true;
}
}
return false;
}

bool ConfigCassandraClient::FQNameReader() {
for (ConfigClientManager::ObjectTypeList::const_iterator it =
mgr()->config_json_parser()->ObjectTypeListToRead().begin();
Expand Down Expand Up @@ -804,6 +822,10 @@ bool ConfigCassandraPartition::IsListOrMapPropEmpty(const string &uuid_key,
return true;
}

bool ConfigCassandraPartition::IsTaskTriggered() const {
return (config_reader_->IsSet());
}

bool ConfigCassandraPartition::ConfigReader() {
CHECK_CONCURRENCY("cassandra::Reader");

Expand Down
4 changes: 2 additions & 2 deletions config-client-mgr/config_cassandra_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class ConfigCassandraPartition {
boost::asio::io_service *ioservice();

bool IsListOrMapPropEmpty(const string &uuid_key, const string &lookup_key);

bool IsTaskTriggered() const;
protected:
virtual bool ReadObjUUIDTable(const std::set<std::string> &uuid_list);
bool ProcessObjUUIDTableEntry(const std::string &uuid_key,
Expand Down Expand Up @@ -268,7 +268,7 @@ class ConfigCassandraClient : public ConfigDbClient {
virtual std::string uuid_str(const std::string &uuid);
virtual bool IsListOrMapPropEmpty(const string &uuid_key,
const string &lookup_key);

virtual bool IsTaskTriggered() const;
protected:
typedef std::pair<std::string, std::string> ObjTypeUUIDType;
typedef std::list<ObjTypeUUIDType> ObjTypeUUIDList;
Expand Down
10 changes: 10 additions & 0 deletions config-client-mgr/config_client_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,16 @@ bool ConfigClientManager::InitConfigClient() {
// Due to this task policy, if the reinit task is running, it ensured
// that above mutually exclusive tasks have finished/aborted
// Perform PostShutdown to prepare for new connection
// However, it is possible that these tasks have been scheduled
// but yet to begin execution. For Task and WorkQueue events, their
// destructor takes core of this but for TaskTrigger events, the
// destructor will crash (See Bug #1786154) as it expects the task
// to not be scheduled. Taking care of that case here by checking
// if TaskTrigger events are scheduled (but not executing) and
// return if they are so that this will be retried.
if (config_db_client_->IsTaskTriggered()) {
return false;
}
PostShutdown();
}
CONFIG_CLIENT_DEBUG(ConfigClientMgrDebug,
Expand Down
3 changes: 3 additions & 0 deletions config-client-mgr/config_db_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,6 @@ int ConfigDbClient::GetFirstConfigDbPort() const {
return !config_db_ports_.empty() ? config_db_ports_[0] : 0;
}

bool ConfigDbClient::IsTaskTriggered() const {
return false;
}
3 changes: 2 additions & 1 deletion config-client-mgr/config_db_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ class ConfigDbClient {
const std::string &last_uuid, uint32_t num_entries,
std::vector<ConfigDBUUIDCacheEntry> *entries) const = 0;

virtual bool IsListOrMapPropEmpty(const std::string &uuid_key,
virtual bool IsListOrMapPropEmpty(const std::string &uuid_key,
const std::string &lookup_key) = 0;
virtual bool IsTaskTriggered() const;
private:
std::string config_db_user_;
std::string config_db_password_;
Expand Down

0 comments on commit 39a4814

Please sign in to comment.