Skip to content

[Mellanox] [saidump] Install missing rdb-cli tool#23990

Merged
liat-grozovik merged 2 commits intosonic-net:masterfrom
nazariig:master-saidump-fix
Nov 9, 2025
Merged

[Mellanox] [saidump] Install missing rdb-cli tool#23990
liat-grozovik merged 2 commits intosonic-net:masterfrom
nazariig:master-saidump-fix

Conversation

@nazariig
Copy link
Collaborator

@nazariig nazariig commented Sep 12, 2025

Propagating: #20948

ERR log:

root@sonic:/home/admin# docker exec -ti syncd bash
root@sonic:/# /usr/bin/saidump.sh
/usr/bin/saidump.sh: line 26: rdb-cli: command not found
JSON parsing error: [json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - unexpected end of input; expected '[', '{', or a literal.

Why I did it

  • To fix dump generation

Work item tracking

  • N/A

How I did it

  • Installed missing rdb-cli tool to syncd docker

How to verify it

  1. Run script saidump.sh
root@sonic:/# /usr/bin/saidump.sh
+ save_saidump_by_rdb
+ local filepath=/var/run/redis/sonic-db/database_config.json
++ python3 -c '
import json
with open('\''/var/run/redis/sonic-db/database_config.json'\'') as json_file:
  data = json.load(json_file)
  print(data['\''INSTANCES'\'']['\''redis'\'']['\''hostname'\''], data['\''INSTANCES'\'']['\''redis'\'']['\''port'\''], data['\''INSTANCES'\'']['\''redis'\'']['\''unix_socket_path'\''])'
+ local 'redis_config=127.0.0.1 6379 /var/run/redis/redis.sock'
+ redis_config=(${redis_config// / })
+ local hostname=127.0.0.1
+ local port=6379
++ dirname /var/run/redis/redis.sock
+ local redis_dir=/var/run/redis
+ logger 'saidump.sh: hostname:127.0.0.1, port:6379, redis_dir:/var/run/redis'
+ logger 'saidump.sh: [1] Get the remote backups of RDB file.'
+ redis-cli -h 127.0.0.1 -p 6379 --rdb /var/run/redis/dump.rdb
+ logger 'saidump.sh: [2] Run rdb-cli command to convert the dump files into JSON files.'
+ rdb-cli /var/run/redis/dump.rdb json
+ tee /var/run/redis/dump.json
+ logger 'saidump.sh: [3] Run saidump -r to get the result at standard output from the JSON file.'
+ saidump -r /var/run/redis/dump.json -m 100
+ logger 'saidump.sh: [4] Clear the temporary files.'
+ rm -f /var/run/redis/dump.rdb
+ rm -f /var/run/redis/dump.json
root@sonic:/# echo $?
0

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

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Tested branch (Please provide the tested image version)

  • master
  • 202405

Description for the changelog

  • N/A

Link to config_db schema for YANG module changes

  • N/A

A picture of a cute animal (not mandatory but encouraged)

      .---.        .-----------
     /     \  __  /    ------
    / /     \(  )/    -----
   //////   ' \/ `   ---
  //// / // :    : ---
 // /   /  /`    '--
//          //..\\
       ====UU====UU====
           '//||\\`
             ''``

@nazariig nazariig requested a review from lguohan as a code owner September 12, 2025 10:25
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@nazariig nazariig added the bugfix the PR is a bug fix PR label Sep 12, 2025
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Collaborator

@saiarcot895 @qiluo-msft @JunhongMao why did we have the rdb-cli vendor specific in the first place #19268
Since it was added only for docker-syncd-brcm the saidump tool now doesn't work for all other vendors and to fix for mellanox platforms this PR is raised.
However I dislike the approach where we have to add this change for every vendor. Can we explore to make it generic?

@liat-grozovik liat-grozovik changed the title [saidump]: Install missing rdb-cli tool [Mellanox] [saidump] Install missing rdb-cli tool Oct 23, 2025
@liat-grozovik
Copy link
Collaborator

@saiarcot895 @qiluo-msft @JunhongMao why did we have the rdb-cli vendor specific in the first place #19268 Since it was added only for docker-syncd-brcm the saidump tool now doesn't work for all other vendors and to fix for mellanox platforms this PR is raised. However I dislike the approach where we have to add this change for every vendor. Can we explore to make it generic?

I see no comment on this and we have currently a problem with sai dump which is a tool to be used.
thus i think we need to have it fixed this way and I totally agree that we need to think of a better approach.
@dgsudharsan is that OK? if so, please open a new github issue that should be discussed and agreed about the proper solution

@liat-grozovik liat-grozovik merged commit 4911691 into sonic-net:master Nov 9, 2025
12 checks passed
ashutosh-agrawal pushed a commit to AnantKishorSharma/sonic-buildimage that referenced this pull request Nov 30, 2025
- Why I did it
To fix dump generation

- How I did it
Installed missing rdb-cli tool to syncd docker

- How to verify it
Run script saidump.sh

---------

Signed-off-by: Nazarii Hnydyn <[email protected]>
FengPan-Frank pushed a commit to FengPan-Frank/sonic-buildimage that referenced this pull request Dec 4, 2025
- Why I did it
To fix dump generation

- How I did it
Installed missing rdb-cli tool to syncd docker

- How to verify it
Run script saidump.sh

---------

Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Feng Pan <[email protected]>
xwjiang-ms pushed a commit to xwjiang-ms/sonic-buildimage that referenced this pull request Dec 22, 2025
- Why I did it
To fix dump generation

- How I did it
Installed missing rdb-cli tool to syncd docker

- How to verify it
Run script saidump.sh

---------

Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: xiaweijiang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants