Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[counters]: separate query of port/queue counters #483

Merged
merged 1 commit into from
Apr 24, 2018
Merged

[counters]: separate query of port/queue counters #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ extern CrmOrch *gCrmOrch;

#define VLAN_PREFIX "Vlan"
#define DEFAULT_VLAN_ID 1
#define FLEX_STAT_COUNTER_POLL_MSECS "1000"
#define STAT_COUNTER_FLEX_COUNTER_GROUP "STAT_COUNTER"
#define PORT_FLEX_STAT_COUNTER_POLL_MSECS "1000"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both polling intervals controlled by cli or through fields in config_db.json?
If yes, can you please tell me how?

If not, then those intervals need to have more control.

Copy link
Contributor

@nikos-github nikos-github Apr 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also queue counters polling must not be enabled by default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nikos-Li , please create feature request in the issue, we'll triage later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan This isn't a feature request, nor a proper solution to the CPU increase we have observed. This needs to be discussed in the 3-way meeting. Please include it in the agenda.

Copy link
Contributor

@nikos-github nikos-github Apr 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan @sihuihan88 Please also provide CPU util data of before and after and a proper root cause analysis as well as the testing done, verification etc. None of this exists in the PR and an almost 200% increase in CPU util, makes this a critical fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lguohan @sihuihan88 Do not merge this without proper RCA and explicit approval and discussion in the 3-way meeting.

#define QUEUE_FLEX_STAT_COUNTER_POLL_MSECS "10000"
#define PORT_STAT_COUNTER_FLEX_COUNTER_GROUP "PORT_STAT_COUNTER"
#define QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP "QUEUE_STAT_COUNTER"

static map<string, sai_port_fec_mode_t> fec_mode_map =
{
Expand Down Expand Up @@ -138,8 +140,11 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
m_flexCounterGroupTable = unique_ptr<ProducerTable>(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE));

vector<FieldValueTuple> fields;
fields.emplace_back(POLL_INTERVAL_FIELD, FLEX_STAT_COUNTER_POLL_MSECS);
m_flexCounterGroupTable->set(STAT_COUNTER_FLEX_COUNTER_GROUP, fields);
fields.emplace_back(POLL_INTERVAL_FIELD, PORT_FLEX_STAT_COUNTER_POLL_MSECS);
m_flexCounterGroupTable->set(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP, fields);

fields.emplace_back(POLL_INTERVAL_FIELD, QUEUE_FLEX_STAT_COUNTER_POLL_MSECS);
m_flexCounterGroupTable->set(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP, fields);

uint32_t i, j;
sai_status_t status;
Expand Down Expand Up @@ -969,9 +974,14 @@ bool PortsOrch::removePort(sai_object_id_t port_id)
return true;
}

string PortsOrch::getFlexCounterTableKey(string key)
string PortsOrch::getPortFlexCounterTableKey(string key)
{
return string(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
}

string PortsOrch::getQueueFlexCounterTableKey(string key)
{
return string(STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
return string(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
}

bool PortsOrch::initPort(const string &alias, const set<int> &lane_set)
Expand Down Expand Up @@ -1007,7 +1017,7 @@ bool PortsOrch::initPort(const string &alias, const set<int> &lane_set)
m_counterTable->set("", fields);

/* Add port to flex_counter for updating stat counters */
string key = getFlexCounterTableKey(sai_serialize_object_id(p.m_port_id));
string key = getPortFlexCounterTableKey(sai_serialize_object_id(p.m_port_id));
std::string delimiter = "";
std::ostringstream counters_stream;
for (const auto &id: portStatIds)
Expand Down Expand Up @@ -1771,7 +1781,7 @@ void PortsOrch::initializeQueues(Port &port)
queueTypeVector.push_back(queueTypeTuple);
}

string key = getFlexCounterTableKey(sai_serialize_object_id(port.m_queue_ids[queueIndex]));
string key = getQueueFlexCounterTableKey(sai_serialize_object_id(port.m_queue_ids[queueIndex]));

std::string delimiter = "";
std::ostringstream counters_stream;
Expand Down
4 changes: 3 additions & 1 deletion orchagent/portsorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class PortsOrch : public Orch, public Subject
unique_ptr<ProducerTable> m_flexCounterTable;
unique_ptr<ProducerTable> m_flexCounterGroupTable;

std:: string getFlexCounterTableKey(std::string s);
std::string getQueueFlexCounterTableKey(std::string s);
std::string getPortFlexCounterTableKey(std::string s);

shared_ptr<DBConnector> m_counter_db;
shared_ptr<DBConnector> m_flex_db;

Expand Down