[eeprom] Use swsscommon instead of redis-py in eeprom_tlvinfo. - #748
Open
jianyuewu wants to merge 1 commit into
Open
[eeprom] Use swsscommon instead of redis-py in eeprom_tlvinfo.#748jianyuewu wants to merge 1 commit into
jianyuewu wants to merge 1 commit into
Conversation
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>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
jianyuewu
requested review from
Junchao-Mellanox,
judyjoseph,
keboliu and
lguohan
August 27, 2026 02:03
keboliu
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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:import redisand theSTATE_DB_INDEXconstant, and droppedredisfromsetup.pyinstall_requires(no longer imported anywhere undersonic_platform_base/).redis_clientproperty tostate_db, returning a lazily connectedSonicV2Connectorbound to STATE_DB. It connects withretry_on=Falseso an unreachable STATE_DB fails fast, matching the old redis client (which raisedConnectionErrorrather than hanging)._redis_hgetreads viaSonicV2Connector.get, which returns a decodedstr, so the previous.decode()is dropped;rstrip('\0')is kept.EepromRedisVisitorwrites via a new_hmsethelper that coerces field values tostr(SonicV2Connector.hmsetrequires str values, whereas redis-py stringified them implicitly), so int fields such asVersionandTotal Lengthare stored exactly as before.from . import sfp_baseinchassis_base.py(referenced nowhere except a docstring).The
EEPROM_INFOkey space and field names are unchanged, so all consumers (show platform syseeprom, gNMI, etc.) are unaffected.How to verify it
show platform syseeprom,decode-syseeprom, anddecode-syseeprom -dreturn identical output before and after;EEPROM_INFO|*contents in STATE_DB are byte-for-byte identical.Which release branch to backport (provide reason below if selected)
Tested branch
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.