Skip to content

psud: Optimize Redis updates by conditionally writing static fields #717

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

Open
GauravNagesh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

psud: Optimize Redis updates by conditionally writing static fields #717

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
52 changes: 30 additions & 22 deletions sonic-psud/scripts/psud
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ class DaemonPsud(daemon_base.DaemonBase):
power_exceeded_threshold = psu_status.power_exceeded_threshold
power_warning_suppress_threshold = try_get(psu.get_psu_power_warning_suppress_threshold, NOT_AVAILABLE)
power_critical_threshold = try_get(psu.get_psu_power_critical_threshold, NOT_AVAILABLE)
power_exceeded_changed = False
if psu_status.check_psu_power_threshold:
# Calculate total power
system_power = float(power)
Expand All @@ -568,7 +569,8 @@ class DaemonPsud(daemon_base.DaemonBase):
# Raise alarm
power_exceeded_threshold = True

if psu_status.set_power_exceed_threshold(power_exceeded_threshold) and not self.psu_threshold_exceeded_logged:
power_exceeded_changed = psu_status.set_power_exceed_threshold(power_exceeded_threshold)
if power_exceeded_changed and not self.psu_threshold_exceeded_logged:
# Since this is a system level PSU power exceeding check, we do not need to log it for each PSU
log_on_status_changed(self, not psu_status.power_exceeded_threshold,
'PSU power warning cleared: system power {} is back to normal, below the warning suppress threshold {}.'.format(system_power, power_warning_suppress_threshold),
Expand All @@ -594,27 +596,33 @@ class DaemonPsud(daemon_base.DaemonBase):
if set_led:
self._set_psu_led(psu, psu_status)

fvs = swsscommon.FieldValuePairs(
[(PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model, NOT_AVAILABLE))),
(PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial, NOT_AVAILABLE))),
(PSU_INFO_REV_FIELD, str(try_get(psu.get_revision, NOT_AVAILABLE))),
(PSU_INFO_TEMP_FIELD, str(temperature)),
(PSU_INFO_TEMP_TH_FIELD, str(temperature_threshold)),
(PSU_INFO_VOLTAGE_FIELD, str(voltage)),
(PSU_INFO_VOLTAGE_MIN_TH_FIELD, str(voltage_low_threshold)),
(PSU_INFO_VOLTAGE_MAX_TH_FIELD, str(voltage_high_threshold)),
(PSU_INFO_CURRENT_FIELD, str(current)),
(PSU_INFO_POWER_FIELD, str(power)),
(PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)),
(PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)),
(PSU_INFO_POWER_OVERLOAD, str(power_exceeded_threshold)),
(PSU_INFO_FRU_FIELD, str(is_replaceable)),
(PSU_INFO_IN_CURRENT_FIELD, str(in_current)),
(PSU_INFO_IN_VOLTAGE_FIELD, str(in_voltage)),
(PSU_INFO_POWER_MAX_FIELD, str(max_power)),
(PSU_INFO_PRESENCE_FIELD, 'true' if _wrapper_get_psu_presence(index) else 'false'),
(PSU_INFO_STATUS_FIELD, 'true' if _wrapper_get_psu_status(index) else 'false'),
])
# Only add rarely-changing fields if changed or first run
fvs = swsscommon.FieldValuePairs()
if presence_changed or self.first_run:
fvs.append((PSU_INFO_MODEL_FIELD, str(try_get(psu.get_model, NOT_AVAILABLE))))
fvs.append((PSU_INFO_SERIAL_FIELD, str(try_get(psu.get_serial, NOT_AVAILABLE))))
fvs.append((PSU_INFO_REV_FIELD, str(try_get(psu.get_revision, NOT_AVAILABLE))))
fvs.append((PSU_INFO_PRESENCE_FIELD, 'true' if presence else 'false'))
fvs.append((PSU_INFO_FRU_FIELD, str(is_replaceable)))
fvs.append((PSU_INFO_TEMP_TH_FIELD, str(temperature_threshold)))
fvs.append((PSU_INFO_VOLTAGE_MIN_TH_FIELD, str(voltage_low_threshold)))
fvs.append((PSU_INFO_VOLTAGE_MAX_TH_FIELD, str(voltage_high_threshold)))
if power_good_changed or self.first_run:
fvs.append((PSU_INFO_STATUS_FIELD, 'true' if power_good else 'false'))
if power_exceeded_changed or self.first_run:
fvs.append((PSU_INFO_POWER_OVERLOAD, str(power_exceeded_threshold)))

# Always adding the frequently-changing fields
fvs.append((PSU_INFO_TEMP_FIELD, str(temperature)))
fvs.append((PSU_INFO_VOLTAGE_FIELD, str(voltage)))
fvs.append((PSU_INFO_CURRENT_FIELD, str(current)))
fvs.append((PSU_INFO_POWER_FIELD, str(power)))
fvs.append((PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)))
fvs.append((PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)))
fvs.append((PSU_INFO_IN_CURRENT_FIELD, str(in_current)))
fvs.append((PSU_INFO_IN_VOLTAGE_FIELD, str(in_voltage)))
fvs.append((PSU_INFO_POWER_MAX_FIELD, str(max_power)))

self.psu_tbl.set(name, fvs)

def _update_psu_entity_info(self):
Expand Down
13 changes: 10 additions & 3 deletions sonic-psud/tests/mocked_libs/swsscommon/swsscommon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,18 @@ def get_size(self):


class FieldValuePairs:
fv_dict = {}

def __init__(self, tuple_list):
if isinstance(tuple_list, list) and isinstance(tuple_list[0], tuple):
def __init__(self, tuple_list=None):
# Mock to support both both constructors FieldValuePairs() and FieldValuePairs(tuple_list)
if tuple_list is None:
self.fv_dict = {}
elif isinstance(tuple_list, list) and len(tuple_list) > 0 and isinstance(tuple_list[0], tuple):
self.fv_dict = dict(tuple_list)
else:
self.fv_dict = dict()

def append(self, kv_tuple):
self.fv_dict[kv_tuple[0]] = kv_tuple[1]

def __setitem__(self, key, kv_tuple):
self.fv_dict[kv_tuple[0]] = kv_tuple[1]
Expand Down
124 changes: 91 additions & 33 deletions sonic-psud/tests/test_DaemonPsud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,28 +143,45 @@ def test_update_psu_data(self):
expected_calls = [mock.call("Failed to update PSU data - Test message")] * 2
assert daemon_psud.log_warning.mock_calls == expected_calls

def _construct_expected_fvp(self, power=100.0, power_warning_suppress_threshold='N/A', power_critical_threshold='N/A', power_overload=False):
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_MODEL_FIELD, 'Fake Model'),
(psud.PSU_INFO_SERIAL_FIELD, '12345678'),
(psud.PSU_INFO_REV_FIELD, '1234'),
(psud.PSU_INFO_TEMP_FIELD, '30.0'),
(psud.PSU_INFO_TEMP_TH_FIELD, '50.0'),
(psud.PSU_INFO_VOLTAGE_FIELD, '12.0'),
(psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, '11.0'),
(psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, '13.0'),
(psud.PSU_INFO_CURRENT_FIELD, '8.0'),
(psud.PSU_INFO_POWER_FIELD, str(power)),
(psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)),
(psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)),
(psud.PSU_INFO_POWER_OVERLOAD, str(power_overload)),
(psud.PSU_INFO_FRU_FIELD, 'True'),
(psud.PSU_INFO_IN_VOLTAGE_FIELD, '220.25'),
(psud.PSU_INFO_IN_CURRENT_FIELD, '0.72'),
(psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'),
(psud.PSU_INFO_PRESENCE_FIELD, 'true'),
(psud.PSU_INFO_STATUS_FIELD, 'true'),
])
def _construct_expected_fvp(self, power=100.0, power_warning_suppress_threshold='N/A', power_critical_threshold='N/A', power_overload=False, first_run=True, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False):
# Build field list based on what should be included
fv_list = []

# Rarely-changing fields (only on first run or when presence changed)
if first_run or presence_changed:
fv_list.extend([
(psud.PSU_INFO_MODEL_FIELD, 'Fake Model'),
(psud.PSU_INFO_SERIAL_FIELD, '12345678'),
(psud.PSU_INFO_REV_FIELD, '1234'),
(psud.PSU_INFO_PRESENCE_FIELD, 'true' if presence else 'false'),
(psud.PSU_INFO_FRU_FIELD, 'True'),
(psud.PSU_INFO_TEMP_TH_FIELD, '50.0'),
(psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, '11.0'),
(psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, '13.0'),
])

# Power good status (only on first run or when power_good changed)
if first_run or power_good_changed:
fv_list.append((psud.PSU_INFO_STATUS_FIELD, 'true' if power_good else 'false'))

# Power overload (only on first run or when power_exceeded changed)
if first_run or power_overload_changed:
fv_list.append((psud.PSU_INFO_POWER_OVERLOAD, str(power_overload)))

# Frequently-changing fields (always included)
fv_list.extend([
(psud.PSU_INFO_TEMP_FIELD, '30.0'),
(psud.PSU_INFO_VOLTAGE_FIELD, '12.0'),
(psud.PSU_INFO_CURRENT_FIELD, '8.0'),
(psud.PSU_INFO_POWER_FIELD, str(power)),
(psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, str(power_warning_suppress_threshold)),
(psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, str(power_critical_threshold)),
(psud.PSU_INFO_IN_CURRENT_FIELD, '0.72'),
(psud.PSU_INFO_IN_VOLTAGE_FIELD, '220.25'),
(psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'),
])

expected_fvp = psud.swsscommon.FieldValuePairs(fv_list)
return expected_fvp

@mock.patch('psud._wrapper_get_psu_presence', mock.MagicMock())
Expand Down Expand Up @@ -205,7 +222,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(100.0, 120.0, 130.0, False)
expected_fvp = self._construct_expected_fvp(100.0, 120.0, 130.0, False, first_run=True, power_overload_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
Expand All @@ -218,7 +235,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, False)
expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
Expand All @@ -229,7 +246,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True)
expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True, first_run=False, power_overload_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
Expand All @@ -240,7 +257,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, True)
expected_fvp = self._construct_expected_fvp(115.0, 120.0, 130.0, True, first_run=False, power_overload_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
Expand All @@ -251,7 +268,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False)
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
daemon_psud._update_led_color()
Expand All @@ -262,7 +279,7 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True)
expected_fvp = self._construct_expected_fvp(125.0, 120.0, 130.0, True, first_run=False, power_overload_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()
Expand All @@ -273,57 +290,98 @@ def test_power_threshold(self):
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False)
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=True, power_good=True, power_good_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()

# PSU power becomes down
psu.set_status(False)
daemon_psud._update_single_psu_data(1, psu)
daemon_psud._update_led_color()
assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=False, power_good_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_RED == psu.get_status_led()

# PSU power becomes up
psu.set_status(True)
daemon_psud._update_single_psu_data(1, psu)
daemon_psud._update_led_color()
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()

# PSU becomes absent
psu.set_presence(False)
daemon_psud._update_single_psu_data(1, psu)
daemon_psud._update_led_color()
assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
# When presence is False, most values become NOT_AVAILABLE
# BUT power thresholds are still fetched from PSU has these data fields according to psu object we created in our testcase above
expected_fvp = psud.swsscommon.FieldValuePairs([
(psud.PSU_INFO_MODEL_FIELD, 'Fake Model'),
(psud.PSU_INFO_SERIAL_FIELD, '12345678'),
(psud.PSU_INFO_REV_FIELD, '1234'),
(psud.PSU_INFO_PRESENCE_FIELD, 'false'),
(psud.PSU_INFO_FRU_FIELD, 'True'),
(psud.PSU_INFO_TEMP_TH_FIELD, 'N/A'),
(psud.PSU_INFO_VOLTAGE_MIN_TH_FIELD, 'N/A'),
(psud.PSU_INFO_VOLTAGE_MAX_TH_FIELD, 'N/A'),
(psud.PSU_INFO_STATUS_FIELD, 'false'),
(psud.PSU_INFO_TEMP_FIELD, 'N/A'),
(psud.PSU_INFO_VOLTAGE_FIELD, 'N/A'),
(psud.PSU_INFO_CURRENT_FIELD, 'N/A'),
(psud.PSU_INFO_POWER_FIELD, 'N/A'),
(psud.PSU_INFO_POWER_WARNING_SUPPRESS_THRESHOLD, '120.0'),
(psud.PSU_INFO_POWER_CRITICAL_THRESHOLD, '130.0'),
(psud.PSU_INFO_IN_CURRENT_FIELD, 'N/A'),
(psud.PSU_INFO_IN_VOLTAGE_FIELD, 'N/A'),
(psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'),
])
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_RED == psu.get_status_led()

# PSU becomes present
psu.set_presence(True)
daemon_psud._update_single_psu_data(1, psu)
daemon_psud._update_led_color()
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 130.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=True, presence=True, presence_changed=True)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)
daemon_psud._update_led_color()
assert psu.STATUS_LED_COLOR_GREEN == psu.get_status_led()

# Thresholds become invalid on the fly
# Critical threshold becomes invalid (NotImplementedError)
psu.get_psu_power_critical_threshold = mock.MagicMock(side_effect=NotImplementedError(''))
daemon_psud._update_single_psu_data(1, psu)
assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
# When threshold becomes N/A, it's still written to the table
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 'N/A', False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

# Critical threshold becomes valid again
psu.get_psu_power_critical_threshold = mock.MagicMock(return_value=120.0)
daemon_psud.psu_status_dict[1].check_psu_power_threshold = True
daemon_psud._update_single_psu_data(1, psu)
assert daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 120.0, 120.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

# Warning threshold becomes invalid (NotImplementedError)
psu.get_psu_power_warning_suppress_threshold = mock.MagicMock(side_effect=NotImplementedError(''))
daemon_psud._update_single_psu_data(1, psu)
assert not daemon_psud.psu_status_dict[1].check_psu_power_threshold
assert not daemon_psud.psu_status_dict[1].power_exceeded_threshold
expected_fvp = self._construct_expected_fvp(105.0, 'N/A', 120.0, False, first_run=False, power_overload_changed=False, power_good=True, power_good_changed=False, presence=True, presence_changed=False)
daemon_psud.psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

def test_set_psu_led(self):
mock_logger = mock.MagicMock()
Expand Down
Loading