Skip to content

use "\n" instead of semicolon to separate redis commands in a c++ string#954

Merged
liuh-80 merged 2 commits intosonic-net:masterfrom
a114j0y:patch-1
Dec 2, 2024
Merged

use "\n" instead of semicolon to separate redis commands in a c++ string#954
liuh-80 merged 2 commits intosonic-net:masterfrom
a114j0y:patch-1

Conversation

@a114j0y
Copy link
Contributor

@a114j0y a114j0y commented Nov 21, 2024

replace ";" with "\n" when concatenating multiple redis cmds

replace ";" with "\n"
@prsunny
Copy link
Contributor

prsunny commented Nov 22, 2024

@liuh-80 , @qiluo-msft , can you merge this?

@a114j0y a114j0y changed the title Update redispipeline.h use backslash n instead of semicolon to separate redis commands in a c++ string Nov 27, 2024
@a114j0y a114j0y changed the title use backslash n instead of semicolon to separate redis commands in a c++ string use "\n" instead of semicolon to separate redis commands in a c++ string Nov 27, 2024
@yuezhoujk
Copy link

/azp run Azure.sonic-swss-common

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 merged commit aa1021f into sonic-net:master Dec 2, 2024
@lguohan
Copy link
Contributor

lguohan commented Dec 2, 2024

@qiluo-msft , @liuh-80 , @yuezhoujk , please explain motivation of such change.

@a114j0y
Copy link
Contributor Author

a114j0y commented Dec 4, 2024

@qiluo-msft , @liuh-80 , @yuezhoujk , please explain motivation of such change.

We see such an exception when running the testcase in the azure pipeline:

C++ exception with description "RedisReply catches system_error: command: *3\r\n$6\r\nSCRIPT\r\n$4\r\nLOAD\r\n$52\r\nredis.call('PUBLISH', 'ROUTE_TABLE_CHANNEL@0', 'G');\r\n, reason: Expected to get redis type 1 got type 3, err: NON-STRING-REPLY: Input/output error: Input/output error" thrown in SetUp().

which indicates that the redis server fails to do SCRIPT LOAD, although it works on our end.

We checked the c++ string m_luaPub that is to be loaded into a redis script:
m_luaPub += "redis.call('PUBLISH', '" + channel + "', 'G');";

it could be successfully loaded into script on our end, but throws exception in the azure pipeline.

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.

7 participants