Skip to content

Commit

Permalink
Correct real time CPU Utilization calculation (sonic-net#173)
Browse files Browse the repository at this point in the history
**What I did**
Correct real time CPU Utilization calculation

**Why I did it**
Now in procdockerstatsd script we iterate all processes every 2 minutes and ingest into state_db, 
https://psutil.readthedocs.io/en/latest/#psutil.cpu_percent, psutil supports both blocking and non-blocking way, and we prefer to use non-blocking way, thus now psutil.cpu_percent() used is to calculate CPU utilization since last time being triggered, current implementation will create new process object in below lines every time:
for process in psutil.process_iter(['pid', 'ppid', 'memory_percent', 'cpu_percent', 'create_time', 'cmdline']):

In order to get correct CPU utilization from last 2 minutes, we need store process object into some variable, so that every iteration will first lookup all_process_obj member and use existing process object, otherwise create new object for further use. And clean up the dead process object in case some process crashes.

**How I verified it**
Verified by UT and will check by ksoftirqd reproduced case.
  • Loading branch information
FengPan-Frank authored Oct 31, 2024
1 parent f95b7cd commit 13a5419
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
28 changes: 26 additions & 2 deletions scripts/procdockerstatsd
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ REDIS_HOSTIP = "127.0.0.1"


class ProcDockerStats(daemon_base.DaemonBase):
all_process_obj = {}

def __init__(self, log_identifier):
super(ProcDockerStats, self).__init__(log_identifier)
Expand Down Expand Up @@ -137,10 +138,25 @@ class ProcDockerStats(daemon_base.DaemonBase):

def update_processstats_command(self):
processdata = []
for process in psutil.process_iter(['pid', 'ppid', 'memory_percent', 'cpu_percent', 'create_time', 'cmdline']):
pid_set = set()

processes_all = []
for process_obj in psutil.process_iter(['pid', 'ppid', 'memory_percent', 'cpu_percent', 'create_time', 'cmdline']):
processes_all.append(process_obj)
sorted_processes = sorted(processes_all, key=lambda x: x.cpu_percent(), reverse=True)
top_processes = sorted_processes[:1024]

for process_obj in top_processes:
try:
pid = process_obj.pid
pid_set.add(pid)
# store process object and reuse for CPU utilization
if pid in self.all_process_obj:
process = self.all_process_obj[pid]
else:
self.all_process_obj[pid] = process_obj
process = process_obj
uid = process.uids()[0]
pid = process.pid
ppid = process.ppid()
mem = process.memory_percent()
cpu = process.cpu_percent()
Expand All @@ -156,6 +172,14 @@ class ProcDockerStats(daemon_base.DaemonBase):
except (psutil.NoSuchProcess, psutil.AccessDenied, psutil.ZombieProcess):
pass

# erase dead process
remove_keys = []
for id in self.all_process_obj:
if id not in pid_set:
remove_keys.append(id)
for id in remove_keys:
del self.all_process_obj[id]

# wipe out all data before updating with new values
self.state_db.delete_all_by_pattern('STATE_DB', 'PROCESS_STATS|*')

Expand Down
12 changes: 10 additions & 2 deletions tests/procdockerstatsd_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,21 @@ def test_update_processstats_command(self):
valid_create_time2 = int((current_time - timedelta(days=2)).timestamp())
# Create a list of mocked processes
mocked_processes = [
MockProcess(uids=[1000], pid=1234, ppid=5678, memory_percent=10.5, cpu_percent=20.5, create_time=valid_create_time1, cmdline=['python', 'script.py'], user_time=1.5, system_time=2.0),
MockProcess(uids=[1000], pid=5678, ppid=0, memory_percent=5.5, cpu_percent=15.5, create_time=valid_create_time2, cmdline=['bash', 'script.sh'], user_time=3.5, system_time=4.0)
MockProcess(uids=[1000], pid=1234, ppid=0, memory_percent=10.5, cpu_percent=99.0, create_time=valid_create_time1, cmdline=['python', 'script.py'], user_time=1.5, system_time=2.0),
MockProcess(uids=[1000], pid=5678, ppid=0, memory_percent=5.5, cpu_percent=15.5, create_time=valid_create_time2, cmdline=['bash', 'script.sh'], user_time=3.5, system_time=4.0),
MockProcess(uids=[1000], pid=3333, ppid=0, memory_percent=5.5, cpu_percent=15.5, create_time=valid_create_time2, cmdline=['bash', 'script.sh'], user_time=3.5, system_time=4.0)
]
mocked_processes2 = [
MockProcess(uids=[1000], pid=1234, ppid=0, memory_percent=10.5, cpu_percent=20.5, create_time=valid_create_time1, cmdline=['python', 'script.py'], user_time=1.5, system_time=2.0),
MockProcess(uids=[1000], pid=6666, ppid=0, memory_percent=5.5, cpu_percent=15.5, create_time=valid_create_time2, cmdline=['bash', 'script.sh'], user_time=3.5, system_time=4.0)
]

with patch("procdockerstatsd.psutil.process_iter", return_value=mocked_processes) as mock_process_iter:
pdstatsd.all_process_obj = {1234: mocked_processes2[0],
6666: mocked_processes2[1]}
pdstatsd.update_processstats_command()
mock_process_iter.assert_called_once()
assert(len(pdstatsd.all_process_obj)== 3)

@patch('procdockerstatsd.getstatusoutput_noshell_pipe', return_value=([0, 0], ''))
def test_update_fipsstats_command(self, mock_cmd):
Expand Down

0 comments on commit 13a5419

Please sign in to comment.