Skip to content

Commit

Permalink
Minor changes
Browse files Browse the repository at this point in the history 
1. add more comments
2. change function name
3. reduce log severity to DEBUG for counter polling to avoid rate limit

Signed-off-by: Stephen Sun <[email protected]>
  • Loading branch information
@stephenxs
stephenxs committed Jan 2, 2025
1 parent ad9fc8e commit 57c81d5
Showing 1 changed file with 19 additions and 10 deletions.
29 changes: 19 additions & 10 deletions syncd/FlexCounter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ class CounterContext : public BaseCounterContext
{
std::map<std::string, vector<StatType>> counter_prefix_map;
std::vector<StatType> default_partition;
mapCountersByPrefix(supportedIds, counter_prefix_map, default_partition);
createCounterBulkChunkSizePerPrefixPartition(supportedIds, counter_prefix_map, default_partition);

for (auto &counterPrefix : counter_prefix_map)
{
Expand All @@ -569,7 +569,7 @@ class CounterContext : public BaseCounterContext
}
}

bool parseCounterPrefixConfigString(
bool parseBulkChunkSizePerPrefixConfigString(
_In_ const std::string& prefixConfigString)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -600,7 +600,7 @@ class CounterContext : public BaseCounterContext
return true;
}

void mapCountersByPrefix(
void createCounterBulkChunkSizePerPrefixPartition(
_In_ const std::vector<StatType>& supportedIds,
_Out_ std::map<std::string, std::vector<StatType>> &counter_prefix_map,
_Out_ std::vector<StatType> &default_partition,
Expand Down Expand Up @@ -665,7 +665,7 @@ class CounterContext : public BaseCounterContext
m_bulkChunkSizePerPrefix = bulkChunkSizePerPrefix;

// No operation if the input string is invalid or no bulk context has been created
if (!parseCounterPrefixConfigString(bulkChunkSizePerPrefix) || m_bulkContexts.empty())
if (!parseBulkChunkSizePerPrefixConfigString(bulkChunkSizePerPrefix) || m_bulkContexts.empty())
{
return;
}
Expand All @@ -682,12 +682,14 @@ class CounterContext : public BaseCounterContext

if (m_counterChunkSizeMapFromPrefix.empty())
{
// There is still no per counter prefix chunk size configured as the chunk size map is still empty.
singleBulkContext.get()->default_bulk_chunk_size = default_bulk_chunk_size;
}
else
{
// Split the counter IDs according to the counter ID prefix mapping and store them into m_bulkContexts
SWSS_LOG_NOTICE("Split counter IDs set by prefix for the first time %s", bulkChunkSizePerPrefix.c_str());
mapCountersByPrefix(allCounterIds, counterChunkSizePerPrefix, defaultPartition, true);
createCounterBulkChunkSizePerPrefixPartition(allCounterIds, counterChunkSizePerPrefix, defaultPartition, true);

for (auto &counterPrefix : counterChunkSizePerPrefix)
{
Expand All @@ -712,6 +714,11 @@ class CounterContext : public BaseCounterContext
}
else if (m_counterChunkSizeMapFromPrefix.empty())
{
// There have been multiple bulk contexts which can result from
// 1. per counter prefix chunk size configuration
// 2. different objects support different counter ID set
// And there is no per counter prefix chunk size configured any more
// Multiple bulk contexts will be merged into one if they share the same object IDs set, which means case (1).
std::set<sai_object_id_t> oid_set;
std::vector<StatType> counter_ids;
std::shared_ptr<BulkContextType> defaultBulkContext;
Expand Down Expand Up @@ -745,6 +752,8 @@ class CounterContext : public BaseCounterContext
}
else
{
// Multiple bulk contexts and per counter prefix chunk size
// Update the chunk size only in this case.
SWSS_LOG_NOTICE("Update bulk chunk size only %s", bulkChunkSizePerPrefix.c_str());

auto counterChunkSizeMapFromPrefix = m_counterChunkSizeMapFromPrefix;
Expand Down Expand Up @@ -870,7 +879,7 @@ class CounterContext : public BaseCounterContext
return;
}

SWSS_LOG_INFO("Before running plugin %s %s", m_instanceId.c_str(), m_name.c_str());
SWSS_LOG_DEBUG("Before running plugin %s %s", m_instanceId.c_str(), m_name.c_str());

std::vector<std::string> idStrings;
idStrings.reserve(m_objectIdsMap.size());
Expand All @@ -891,7 +900,7 @@ class CounterContext : public BaseCounterContext
m_plugins.end(),
[&] (auto &sha) { runRedisScript(counters_db, sha, idStrings, argv); });

SWSS_LOG_INFO("After running plugin %s %s", m_instanceId.c_str(), m_name.c_str());
SWSS_LOG_DEBUG("After running plugin %s %s", m_instanceId.c_str(), m_name.c_str());
}

bool hasObject() const override
Expand Down Expand Up @@ -1002,7 +1011,7 @@ class CounterContext : public BaseCounterContext
}
uint32_t current = 0;

SWSS_LOG_INFO("Before getting bulk %s %s %s size %u bulk chunk size %u current %u", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str(), size, bulk_chunk_size, current);
SWSS_LOG_DEBUG("Before getting bulk %s %s %s size %u bulk chunk size %u current %u", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str(), size, bulk_chunk_size, current);

while (current < size)
{
Expand All @@ -1022,7 +1031,7 @@ class CounterContext : public BaseCounterContext
}
current += bulk_chunk_size;

SWSS_LOG_INFO("After getting bulk %s %s %s index %u(advanced to %u) bulk chunk size %u", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str(), current - bulk_chunk_size, current, bulk_chunk_size);
SWSS_LOG_DEBUG("After getting bulk %s %s %s index %u(advanced to %u) bulk chunk size %u", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str(), current - bulk_chunk_size, current, bulk_chunk_size);

if (size - current < bulk_chunk_size)
{
Expand Down Expand Up @@ -1051,7 +1060,7 @@ class CounterContext : public BaseCounterContext
values.clear();
}

SWSS_LOG_INFO("After pushing db %s %s %s", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str());
SWSS_LOG_DEBUG("After pushing db %s %s %s", m_instanceId.c_str(), m_name.c_str(), ctx.name.c_str());
}

auto getBulkStatsContext(
Expand Down

0 comments on commit 57c81d5

Please sign in to comment.