Skip to content

[eeprom] Use swsscommon instead of redis-py in eeprom_tlvinfo. - #748

Open
jianyuewu wants to merge 1 commit into
sonic-net:masterfrom
jianyuewu:master_pmon-lazy-import-mem
Open

[eeprom] Use swsscommon instead of redis-py in eeprom_tlvinfo.#748
jianyuewu wants to merge 1 commit into
sonic-net:masterfrom
jianyuewu:master_pmon-lazy-import-mem

Conversation

@jianyuewu

Copy link
Copy Markdown
Contributor

Why I did it

The TlvInfo EEPROM decoder imported redis-py at module top level.
redis-py is heavy (~20MB RSS on import), and this cost is paid by every process that imports the decoder, even those that only decode the EEPROM buffer and never touch the DB.

Work item tracking
  • Microsoft ADO (number only): N/A

How I did it

Replaced redis-py with swsscommon's SonicV2Connector, which is already loaded in these daemons and is the standard SONiC DB accessor:

  • Dropped the top-level import redis and the STATE_DB_INDEX constant, and dropped redis from setup.py install_requires (no longer imported anywhere under sonic_platform_base/).
  • Renamed the redis_client property to state_db, returning a lazily connected SonicV2Connector bound to STATE_DB. It connects with retry_on=False so an unreachable STATE_DB fails fast, matching the old redis client (which raised ConnectionError rather than hanging).
  • _redis_hget reads via SonicV2Connector.get, which returns a decoded str, so the previous .decode() is dropped; rstrip('\0') is kept.
  • EepromRedisVisitor writes via a new _hmset helper that coerces field values to str (SonicV2Connector.hmset requires str values, whereas redis-py stringified them implicitly), so int fields such as Version and Total Length are stored exactly as before.
  • Also removed an unused from . import sfp_base in chassis_base.py (referenced nowhere except a docstring).

The EEPROM_INFO key space and field names are unchanged, so all consumers (show platform syseeprom, gNMI, etc.) are unaffected.

How to verify it

  • On device manual test: show platform syseeprom, decode-syseeprom, and decode-syseeprom -d return identical output before and after; EEPROM_INFO|* contents in STATE_DB are byte-for-byte identical.
daemon before (MB) after (MB) diff
syseepromd 24.5 15.3 -9.2 (-37%)
  • Platform regression test: all passed.

Which release branch to backport (provide reason below if selected)

Tested branch

  • master_RC

Test result

master_RC: PASSED (unit tests + on-device syseeprom output / STATE_DB comparison)

Description for the changelog

[eeprom] Use swsscommon instead of redis-py in eeprom_tlvinfo to drop the redis-py import from every process that decodes the syseeprom.

The TlvInfo EEPROM decoder imported redis-py at module top level and
created a redis.Redis(db=STATE_DB_INDEX) client to read/write the
STATE_DB EEPROM_INFO tables. redis-py is heavy (~20MB RSS on import),
and this cost is paid by every process that imports the decoder, even
those that only decode the EEPROM buffer and never touch the DB.

Replace it with swsscommon's SonicV2Connector, which is already loaded
in these daemons and is the standard SONiC DB accessor. The connector
is created lazily on first use, the same timing as the old client.

Changes.

- Drop the top-level "import redis" and the STATE_DB_INDEX constant, and
  drop redis from setup.py install_requires (no longer imported).
- Rename the redis_client property to state_db, returning a lazily
  connected SonicV2Connector bound to STATE_DB. It connects with
  retry_on=False so an unreachable STATE_DB fails fast, matching the
  old redis client which raised ConnectionError rather than hanging.
- _redis_hget reads via SonicV2Connector.get, which returns a decoded
  str, so the previous .decode() is dropped; rstrip('\0') is kept.
- EepromRedisVisitor writes via a new _hmset helper that coerces field
  values to str (SonicV2Connector.hmset requires str values, whereas
  redis-py stringified them implicitly), so int fields such as Version
  and Total Length are stored exactly as before. Unit tests assert the
  str coercion and inject a mock connector so they never touch a real DB.

The EEPROM_INFO key space and field names are unchanged, so all
consumers (show platform syseeprom, gNMI, etc.) are unaffected.

Also remove an unused "from . import sfp_base" in chassis_base.py; the
symbol is referenced nowhere except a docstring.

Verified on switch: syseepromd process RSS dropped from ~47MB to ~35MB,
EEPROM_INFO contents identical before and after, and redis is no longer
loaded in the process.

Signed-off-by: Jianyue Wu <jianyuew@nvidia.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants