Skip to content
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

Jump to bottom

sonic-db-cli: Don't use unix socket for redis_chassis.server #873

Merged
merged 1 commit into from
Jun 12, 2024
Merged

sonic-db-cli: Don't use unix socket for redis_chassis.server #873

merged 1 commit into from
Jun 12, 2024

Conversation

arista-nwolfe
Copy link
Contributor

Described in sonic-net/sonic-buildimage#18733 we see that swss.sh will try to access CHASSIS_APP_DB via sonic-db-cli.
If we're on a multi-asic system it will include the -n <asic> argument.
Having the -n <asic> argument causes sonic-db-cli to use unix sockets due to #797
If we use a unix socket to try to reach the supervisor from the LC the command will fail with:

admin@cmp217-5:/var/log$ sudo sonic-db-cli -n asic0 CHASSIS_APP_DB keys "*"
Invalid database name input : 'CHASSIS_APP_DB'
Unable to connect to redis (unix-socket): Cannot assign requested address

#866 fixed a similar issue with database.sh by forcing a tcp connection when accessing CHASSIS_APP_DB (redis_chassis.server) specifically for the PING SAVE and FLUSHALL operations.
This change provides that same tcp connection override for the other operations.

@arista-nwolfe
Copy link
Contributor Author

Tagging @mlok-nokia to confirm if this is a viable solution to the CHASSIS_APP_DB unix socket issue on multi-asic LCs we discussed in the meeting today.

@arista-nwolfe arista-nwolfe changed the title sonic-db-cli: Don't use unix socket on for redis_chassis.server sonic-db-cli: Don't use unix socket for redis_chassis.server May 1, 2024
@mlok-nokia
Copy link
Contributor

Tagging @mlok-nokia to confirm if this is a viable solution to the CHASSIS_APP_DB unix socket issue on multi-asic LCs we discussed in the meeting today.

This fix looks good to me. It uses the same mechanism as how it has been handled in the local host. I think we can use this fix to make sonic-db-cli command behaves the same as 202205 branch.

mlok-nokia
mlok-nokia approved these changes May 2, 2024
View reviewed changes
Copy link
Contributor

@mlok-nokia mlok-nokia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

@rlhui rlhui added the P0 label May 15, 2024
@rlhui
Copy link

rlhui commented May 15, 2024

@qiluo-msft ping again, thx

@arista-nwolfe arista-nwolfe force-pushed the master-sonic-db-cli-chassis-app-db branch from b440ed9 to c7dcd0d Compare May 15, 2024 17:14
@qiluo-msft
Copy link
Contributor

sonic-db-cli is behaving similar to redis-cli. If user provide options.m_unixsocket, we should stick with it. Even in existing code, I do not understand why options.m_unixsocket could be changed.

@arlakshm
Copy link
Contributor

arlakshm commented May 17, 2024 •
edited
Loading

sonic-db-cli is behaving similar to redis-cli. If user provide options.m_unixsocket, we should stick with it. Even in existing code, I do not understand why options.m_unixsocket could be changed.

@qiluo-msft in the PR, #797, change was made to always use unix socket when passing the namespace option to the sonic-db-cli command.
The user option is overwritten to use unixsocket in this case

This PR is fixing communication to chassis redis instance from namespace in the linecard, because we cannot use unixsocket in this case.

@lguohan
Copy link
Contributor

lguohan commented May 29, 2024

@qiluo-msft , can you check this one?

@judyjoseph
Copy link
Collaborator

@arista-nwolfe could you rebase this branch

@arista-nwolfe arista-nwolfe force-pushed the master-sonic-db-cli-chassis-app-db branch from c7dcd0d to d8330cf Compare June 5, 2024 17:08
@judyjoseph judyjoseph merged commit e37bfea into sonic-net:master Jun 12, 2024
17 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-swss-common that referenced this pull request Jun 12, 2024
@arista-nwolfe @mssonicbld
Don't use unix socket on for redis_chassis.server (sonic-net#873)
cd8dc7d
@mssonicbld mssonicbld mentioned this pull request Jun 12, 2024
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

5 similar comments
@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!!
#883

arfeigin pushed a commit to arfeigin/sonic-swss-common that referenced this pull request Jun 27, 2024
@arista-nwolfe @arfeigin
Don't use unix socket on for redis_chassis.server (sonic-net#873)
d65b47a
@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!! Auto cherry pick PR will be closed in 3 days.
#883

4 similar comments
@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!! Auto cherry pick PR will be closed in 3 days.
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!! Auto cherry pick PR will be closed in 3 days.
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!! Auto cherry pick PR will be closed in 3 days.
#883

@mssonicbld
Copy link
Collaborator

@arista-nwolfe cherry pick PR didn't pass PR checker. Please check!!! Auto cherry pick PR will be closed in 3 days.
#883

@qiluo-msft
Copy link
Contributor

@arista-nwolfe Could you check why #883 is failing

@arista-nwolfe
Copy link
Contributor Author

@arista-nwolfe Could you check why #883 is failing

I was told that there is a dependency issue in sonic-swss-common on 202405 causing all builds to fail.
Tagging @arlakshm here.

I'm not sure why the 202405 cast was closed.

arista-nwolfe added a commit to arista-nwolfe/sonic-swss-common that referenced this pull request Jul 24, 2024
@arista-nwolfe
Don't use unix socket on for redis_chassis.server (sonic-net#873)
8d6d1d8
@arista-nwolfe arista-nwolfe mentioned this pull request Jul 24, 2024
Closed
@wenyiz2021 wenyiz2021 mentioned this pull request Aug 21, 2024
Closed
wenyiz2021 pushed a commit that referenced this pull request Aug 22, 2024
@mssonicbld
Don't use unix socket on for redis_chassis.server (#873) (#883)
4fecc2f 
Cherry pick PR 873 into 202405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlok-nokia mlok-nokia mlok-nokia approved these changes

@liuh-80 liuh-80 liuh-80 approved these changes

@arlakshm arlakshm arlakshm approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft

Assignees
No one assigned
Labels
Approved for 202405 Branch Created PR to 202405 Branch Included in 202405 Branch P0 Request for 202405 Branch
Projects
SONiC Chassis
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@arista-nwolfe @mlok-nokia @rlhui @qiluo-msft @arlakshm @lguohan @judyjoseph @mssonicbld @liuh-80 @bingwang-ms