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.

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

Re-write sonic-db-cli with c++ for sonic startup performance issue #607

Merged
merged 21 commits into from
May 26, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
SUBDIRS = common pyext tests
SUBDIRS = common pyext sonic-db-cli tests

ACLOCAL_AMFLAGS = -I m4
5 changes: 5 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3.
CFLAGS="$CFLAGS $LIBNL_CFLAGS"
LIBS="$LIBS $LIBNL_LIBS"

dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros
AX_BOOST_BASE(,, [AC_MSG_ERROR([boost lib missing])])
AX_BOOST_PROGRAM_OPTIONS

LDFLAGS="-Wl,--no-undefined $LDFLAGS"
AC_SUBST([LDFLAGS])

Expand Down Expand Up @@ -98,6 +102,7 @@ AC_CONFIG_FILES([
pyext/Makefile
pyext/py2/Makefile
pyext/py3/Makefile
sonic-db-cli/Makefile
tests/Makefile
])

Expand Down
6 changes: 6 additions & 0 deletions debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,9 @@ Architecture: any
Depends: ${shlibs:Depends}, ${misc:Pre-Depends}
Section: libs
Description: This package contains Switch State Service common Python3 library.

Package: sonic-db-cli
Architecture: any
Depends: ${shlibs:Depends}, ${misc:Pre-Depends}
Section: libs
Description: This package contains SONiC DB cli.
1 change: 1 addition & 0 deletions debian/sonic-db-cli.dirs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
usr/bin
1 change: 1 addition & 0 deletions debian/sonic-db-cli.install
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
usr/bin/sonic-db-cli
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 14 additions & 0 deletions sonic-db-cli/Makefile.am
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
INCLUDES = -I $(top_srcdir)

if DEBUG
DBGFLAGS = -ggdb -DDEBUG
else
DBGFLAGS = -g -DNDEBUG
endif

bin_PROGRAMS = sonic-db-cli
sonic_db_cli_SOURCES = sonic-db-cli.h sonic-db-cli.cpp
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved

sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON)
sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON)
sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB)
278 changes: 278 additions & 0 deletions sonic-db-cli/sonic-db-cli.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
#include <iostream>
#include "sonic-db-cli.h"

using namespace swss;
using namespace std;

static string g_netns;
static string g_dbOrOperation;
const char* emptyStr = "";
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved

void printUsage(const po::options_description &allOptions)
{
cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << endl;
cout << allOptions << endl;

cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << endl;
cout << "" << endl;
cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << endl;
cout << "Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10" << endl;
cout << "Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu" << endl;
cout << "Example 4: sonic-db-cli -n asic3 APPL_DB EVAL \"return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}\" 2 k1 k2 v1 v2" << endl;
cout << "Example 5: sonic-db-cli PING | sonic-db-cli -s PING" << endl;
cout << "Example 6: sonic-db-cli SAVE | sonic-db-cli -s SAVE" << endl;
cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << endl;
}

void printRedisReply(redisReply* reply)
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
{
switch(reply->type)
{
case REDIS_REPLY_INTEGER:
cout << reply->integer;
break;
case REDIS_REPLY_STRING:
case REDIS_REPLY_ERROR:
case REDIS_REPLY_STATUS:
case REDIS_REPLY_NIL:
cout << string(reply->str, reply->len);
break;
case REDIS_REPLY_ARRAY:
for (size_t i = 0; i < reply->elements; i++)
{
printRedisReply(reply->element[i]);
}
break;
default:
cerr << reply->type << endl;
throw runtime_error("Unexpected reply type");
}

cout << endl;
}

string buildRedisOperation(vector<string>& commands)
{
int argc = (int)commands.size();
const char** argv = new const char*[argc];
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
size_t* argvc = new size_t[argc];
for (int i = 0; i < argc; i++)
{
argv[i] = strdup(commands[i].c_str());
argvc[i] = commands[i].size();
}

RedisCommand command;
command.formatArgv(argc, argv, argvc);

for (int i = 0; i < argc; i++)
{
free(const_cast<char*>(argv[i]));
}
delete[] argv;
delete[] argvc;

return string(command.c_str());
}

int connectDbInterface(
DBInterface& dbintf,
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
const string& db_name,
const string& netns,
bool isTcpConn)
{
try
{
if (isTcpConn)
{
auto db_hostname = SonicDBConfig::getDbHostname(db_name, netns);
auto db_port = SonicDBConfig::getDbPort(db_name, netns);
dbintf.set_redis_kwargs("", db_hostname, db_port);
}
else
{
auto db_socket = SonicDBConfig::getDbSock(db_name);
dbintf.set_redis_kwargs(db_socket, "", 0);
}

int db_id = SonicDBConfig::getDbId(db_name, netns);
dbintf.connect(db_id, db_name, true);
}
catch (const exception& e)
{
cerr << "Invalid database name input: " << db_name << endl;
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
cerr << e.what() << endl;
return 1;
}

return 0;
}

int handleSingleOperation(
const string& netns,
const string& db_name,
const string& operation,
bool isTcpConn)
{
DBInterface dbintf;
// Need connect DBInterface first then get redis client, because redis client is data member of DBInterface
if (connectDbInterface(dbintf, db_name, netns, isTcpConn) != 0)
{
return 1;
}
auto& client = dbintf.get_redis_client(db_name);

RedisReply reply(&client, operation);
auto replyContext = reply.getContext();
printRedisReply(replyContext);

return 0;
}

int executeCommands(
const string& db_name,
vector<string>& commands,
const string& netns,
bool isTcpConn)
{
auto operation = buildRedisOperation(commands);
return handleSingleOperation(db_name, operation, netns, isTcpConn);
}

int handleAllInstances(
const string& netns,
const string& operation,
bool isTcpConn)
{
auto db_names = SonicDBConfig::getDbList(netns);
for (auto& db_name : db_names)
{
int result = handleSingleOperation(netns, db_name, operation, isTcpConn);
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
if (result != 0)
{
// Stop when any operation failed
return result;
}
}

return 0;
}

int handleOperation(
const po::options_description &allOptions,
const po::variables_map &variablesMap)
{
if (variablesMap.count("db_or_op"))
{
auto dbOrOperation = variablesMap["db_or_op"].as<string>();
auto netns = variablesMap["--namespace"].as<string>();
bool isTcpConn = variablesMap.count("--unixsocket") == 0;
if (netns != "None")
{
SonicDBConfig::initializeGlobalConfig(netns);
}
else
{
// Use the tcp connectivity if namespace is local and unixsocket cmd_option is present.
isTcpConn = true;
}

if (variablesMap.count("cmd"))
{
auto commands = variablesMap["cmd"].as< vector<string> >();
return executeCommands(dbOrOperation, commands, netns, isTcpConn);
}
else if (netns.compare("PING") == 0 || netns.compare("SAVE") == 0 || netns.compare("FLUSHALL") == 0)
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
{
// redis-cli doesn't depend on database_config.json which could raise some exceptions
// sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG'
return handleAllInstances(netns, dbOrOperation, isTcpConn);
}
else
{
printUsage(allOptions);
}
}
else
{
printUsage(allOptions);
}

return 0;
}

void parseCliArguments(
int argc,
char** argv,
po::options_description &allOptions,
po::variables_map &variablesMap)
{
allOptions.add_options()
("--help,-h", "Help message")
("--unixsocket,-s", "Override use of tcp_port and use unixsocket")
("--namespace,-n", po::value<string>(&g_netns)->default_value("None"), "Namespace string to use asic0/asic1.../asicn")
("db_or_op", po::value<string>(&g_dbOrOperation)->default_value(emptyStr), "Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)")
("cmd", po::value< vector<string> >(), "Command to execute in database")
;

po::positional_options_description positional_opts;
positional_opts.add("db_or_op", 1);
positional_opts.add("cmd", -1);

po::store(po::command_line_parser(argc, argv)
.options(allOptions)
.positional(positional_opts)
.run(),
variablesMap);
po::notify(variablesMap);
}

#ifdef TESTING
int sonic_db_cli(int argc, char** argv)
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
#else
int main(int argc, char** argv)
#endif
{
po::options_description allOptions("SONiC DB CLI");
po::variables_map variablesMap;

try
{
parseCliArguments(argc, argv, allOptions, variablesMap);
}
catch (po::error_with_option_name& e)
{
cerr << "Command Line Syntax Error: " << e.what() << endl;
printUsage(allOptions);
return -1;
}
catch (po::error& e)
{
cerr << "Command Line Error: " << e.what() << endl;
printUsage(allOptions);
return -1;
}

if (variablesMap.count("--help"))
{
printUsage(allOptions);
return 0;
}

try
{
return handleOperation(allOptions, variablesMap);
}
catch (const exception& e)
{
cerr << "An exception of type " << e.what() << " occurred. Arguments:" << endl;
for (int idx = 0; idx < argc; idx++)
{
cerr << argv[idx] << " ";
}
cerr << endl;
return 1;
}

return 0;
}
53 changes: 53 additions & 0 deletions sonic-db-cli/sonic-db-cli.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#pragma once

#include <string>
#include <vector>
#include <boost/program_options.hpp>
#include "common/dbconnector.h"
#include "common/dbinterface.h"
#include "common/redisreply.h"

namespace po = boost::program_options;

void printUsage(const po::options_description &all_options);

void printRedisReply(swss::RedisReply& reply);

std::string buildRedisOperation(std::vector<std::string>& commands);

int connectDbInterface(
swss::DBInterface& dbintf,
const std::string& db_name,
const std::string& netns,
bool isTcpConn);

int executeCommands(
const std::string& db_name,
std::vector<std::string>& commands,
const std::string& netns,
bool isTcpConn);

int handleSingleOperation(
const std::string& netns,
const std::string& db_name,
const std::string& operation,
bool isTcpConn);

int handleAllInstances(
const std::string& netns,
const std::string& operation,
bool isTcpConn);

int handleOperation(
const po::options_description &all_options,
const po::variables_map &variables_map);

void parseCliArguments(
int argc,
char** argv,
po::options_description &all_options,
po::variables_map &variables_map);

#ifdef TESTING
int sonic_db_cli(int argc, char** argv);
#endif
Loading