Skip to content

redisclient exists, hmset, hgetallordered function, warm restart table names and table getDbId() function #208

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

Merged
qiluo-msft merged 11 commits into from
Jul 20, 2018
Merged

redisclient exists, hmset, hgetallordered function, warm restart table names and table getDbId() function #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
151be00
Add hmset and hgetallordered redis api function support
jipanyang May 10, 2018
77352f6
Add exists command support for redis client
jipanyang May 29, 2018
2939e6a
Add warm restart table names for appDB and configDB
jipanyang Jul 6, 2018
5fe4e57
Add getDbID() function for tableBase
jipanyang Jul 14, 2018
c1414ed
Address review comments
jipanyang Jul 17, 2018
bbff96a
Remove redundant empty check
jipanyang Jul 17, 2018
b1f270e
For RedisClient exists, make sure no space or tab in key
jipanyang Jul 19, 2018
dc7f42d
Implement template hgetall
qiluo-msft Jul 20, 2018
58dacdd
Implement template shmset.formatHMSET()
qiluo-msft Jul 20, 2018
876a991
Merge pull request #2 from qiluo-msft/jipan/warm_reboot_collab
jipanyang Jul 20, 2018
0d24fd3
Fix build (#3)
qiluo-msft Jul 20, 2018
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
25 changes: 15 additions & 10 deletions common/redisclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ int64_t RedisClient::del(const string &key)
return r.getContext()->integer;
}

bool RedisClient::exists(const string &key)
{
RedisCommand rexists;
if (key.find_first_of(" \t") != string::npos)
Copy link
Copy Markdown
Contributor

@pavel-shirshov pavel-shirshov Jul 20, 2018 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

Probably it's better to trim spaces and tabs from the key? #Resolved

Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Jul 20, 2018

Choose a reason for hiding this comment

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

Actually this input validation is to prevent any input like "key1 key2". Not for conveniently trimming.

In reply to: 204127076 [](ancestors = 204127076)

Copy link
Copy Markdown
Contributor

@pavel-shirshov pavel-shirshov Jul 20, 2018

Choose a reason for hiding this comment

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

I'm good. Thank you for the explanations. I'd a comment explaining why do we need this validation here.

{
SWSS_LOG_ERROR("EXISTS failed, invalid space or tab in single key: %s", key.c_str());
throw runtime_error("EXISTS failed, invalid space or tab in single key");
}
rexists.format("EXISTS %s", key.c_str());
RedisReply r(m_db, rexists, REDIS_REPLY_INTEGER);
return (r.getContext()->integer > 0);
}

int64_t RedisClient::hdel(const string &key, const string &field)
{
RedisCommand shdel;
Expand All @@ -44,16 +57,8 @@ void RedisClient::set(const string &key, const string &value)

unordered_map<string, string> RedisClient::hgetall(const string &key)
{
RedisCommand sincr;
sincr.format("HGETALL %s", key.c_str());
RedisReply r(m_db, sincr, REDIS_REPLY_ARRAY);

auto ctx = r.getContext();

unordered_map<string, string> map;
for (unsigned int i = 0; i < ctx->elements; i += 2)
map[string(ctx->element[i]->str)] = string(ctx->element[i+1]->str);

hgetall(key, std::inserter(map, map.end()));
return map;
}

Expand Down Expand Up @@ -94,7 +99,7 @@ shared_ptr<string> RedisClient::get(const string &key)
sget.format("GET %s", key.c_str());
RedisReply r(m_db, sget);
auto reply = r.getContext();

if (reply->type == REDIS_REPLY_NIL)
{
return shared_ptr<string>(NULL);
Expand Down
32 changes: 31 additions & 1 deletion common/redisclient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ class RedisClient

int64_t del(const std::string &key);

bool exists(const std::string &key);

int64_t hdel(const std::string &key, const std::string &field);

std::unordered_map<std::string, std::string> hgetall(const std::string &key);

template <typename OutputIterator>
void hgetall(const std::string &key, OutputIterator result);

std::vector<std::string> keys(const std::string &key);

std::vector<std::string> hkeys(const std::string &key);
Expand All @@ -40,7 +45,8 @@ class RedisClient

void mset(const std::unordered_map<std::string, std::string> &map);

void hmset(const std::string &key, const std::unordered_map<std::string, std::string> &map);
template<typename InputIterator>
void hmset(const std::string &key, InputIterator start, InputIterator stop);

std::shared_ptr<std::string> get(const std::string &key);

Expand All @@ -62,6 +68,30 @@ class RedisClient
swss::DBConnector *m_db;
};

template<typename OutputIterator>
void RedisClient::hgetall(const std::string &key, OutputIterator result)
{
RedisCommand sincr;
sincr.format("HGETALL %s", key.c_str());
RedisReply r(m_db, sincr, REDIS_REPLY_ARRAY);

auto ctx = r.getContext();

for (unsigned int i = 0; i < ctx->elements; i += 2)
{
*result = std::make_pair(ctx->element[i]->str, ctx->element[i+1]->str);
++result;
}
}

template<typename InputIterator>
void RedisClient::hmset(const std::string &key, InputIterator start, InputIterator stop)
{
RedisCommand shmset;
shmset.formatHMSET(key, start, stop);
RedisReply r(m_db, shmset, REDIS_REPLY_STATUS);
}

}

#endif // __REDISCLIENT_H__
16 changes: 2 additions & 14 deletions common/rediscommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,11 @@ void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen
}
}

/* Format HMSET key multiple field value command */
/* Format HMSET key multiple field value command */
void RedisCommand::formatHMSET(const std::string &key,
const std::vector<FieldValueTuple> &values)
{
if (values.empty()) throw std::invalid_argument("empty values");

const char* cmd = "HMSET";

std::vector<const char*> args = { cmd, key.c_str() };

for (const auto &fvt: values)
{
args.push_back(fvField(fvt).c_str());
args.push_back(fvValue(fvt).c_str());
}

formatArgv((int)args.size(), args.data(), NULL);
formatHMSET(key, values.begin(), values.end());
}

/* Format HSET key field value command */
Expand Down
26 changes: 26 additions & 0 deletions common/rediscommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,16 @@ class RedisCommand {
void formatArgv(int argc, const char **argv, const size_t *argvlen);

/* Format HMSET key multiple field value command */
#ifndef SWIG
__attribute__((deprecated))
#endif
void formatHMSET(const std::string &key,
const std::vector<FieldValueTuple> &values);

template<typename InputIterator>
void formatHMSET(const std::string &key,
InputIterator start, InputIterator stop);

/* Format HSET key field value command */
void formatHSET(const std::string& key, const std::string& field,
const std::string& value);
Expand All @@ -45,4 +52,23 @@ class RedisCommand {
char *temp;
};

template<typename InputIterator>
void RedisCommand::formatHMSET(const std::string &key,
InputIterator start, InputIterator stop)
{
if (start == stop) throw std::invalid_argument("empty values");

const char* cmd = "HMSET";

std::vector<const char*> args = { cmd, key.c_str() };

for (auto i = start; i != stop; i++)
{
args.push_back(fvField(*i).c_str());
args.push_back(fvValue(*i).c_str());
}

formatArgv((int)args.size(), args.data(), NULL);
}

}
2 changes: 1 addition & 1 deletion common/table.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void Table::set(const string &key, const vector<FieldValueTuple> &values,
return;

RedisCommand cmd;
cmd.formatHMSET(getKeyName(key), values);
cmd.formatHMSET(getKeyName(key), values.begin(), values.end());

m_pipe->push(cmd, REDIS_REPLY_STATUS);
if (!m_buffered)
Expand Down
4 changes: 3 additions & 1 deletion common/table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ typedef std::map<std::string,TableMap> TableDump;
class TableBase {
public:
TableBase(int dbId, const std::string &tableName)
: m_tableName(tableName)
: m_tableName(tableName), m_dbId(dbId)
{
/* Look up table separator for the provided DB */
auto it = tableNameSeparatorMap.find(dbId);
Expand All @@ -52,6 +52,7 @@ class TableBase {
}

std::string getTableName() const { return m_tableName; }
int getDbId() const { return m_dbId; }

/* Return the actual key name as a combination of tableName<table_separator>key */
std::string getKeyName(const std::string &key)
Expand All @@ -74,6 +75,7 @@ class TableBase {

std::string m_tableName;
std::string m_tableSeparator;
int m_dbId;
};

class TableEntryWritable {
Expand Down
98 changes: 98 additions & 0 deletions tests/redis_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/selectableevent.h"
#include "common/selectabletimer.h"
#include "common/table.h"
#include "common/redisclient.h"

using namespace std;
using namespace swss;
Expand Down Expand Up @@ -243,6 +244,103 @@ void TableBasicTest(string tableName)
cout << "Done." << endl;
}

TEST(DBConnector, RedisClient)
{
DBConnector db(TEST_DB, "localhost", 6379, 0);

RedisClient redic(&db);

clearDB();
cout << "Starting table manipulations" << endl;

string key_1 = "a";
string key_2 = "b";
vector<FieldValueTuple> values;

for (int i = 1; i < 4; i++)
{
string field = "field_" + to_string(i);
string value = to_string(i);
values.push_back(make_pair(field, value));
}

cout << "- Step 1. SET" << endl;
cout << "Set key [a] field_1:1 field_2:2 field_3:3" << endl;
cout << "Set key [b] field_1:1 field_2:2 field_3:3" << endl;

redic.hmset(key_1, values.begin(), values.end());
redic.hmset(key_2, values.begin(), values.end());

cout << "- Step 2. GET_TABLE_KEYS" << endl;
auto keys = redic.keys("*");
EXPECT_EQ(keys.size(), (size_t)2);

for (auto k : keys)
{
cout << "Get key [" << k << "]" << flush;
EXPECT_EQ(k.length(), (size_t)1);
}

cout << "- Step 3. GET_TABLE_CONTENT" << endl;

for (auto k : keys)
{
cout << "Get key [" << k << "]" << flush;
auto fvs = redic.hgetall(k);
unsigned int size_v = 3;
EXPECT_EQ(fvs.size(), size_v);
for (auto fv: fvs)
{
string value_1 = "1", value_2 = "2";
cout << " " << fvField(fv) << ":" << fvValue(fv) << flush;
if (fvField(fv) == "field_1")
{
EXPECT_EQ(fvValue(fv), value_1);
}
if (fvField(fv) == "field_2")
{
EXPECT_EQ(fvValue(fv), value_2);
}
}
cout << endl;
}

cout << "- Step 4. DEL" << endl;
cout << "Delete key [a]" << endl;
redic.del(key_1);

cout << "- Step 5. GET" << endl;
cout << "Get key [a] and key [b]" << endl;
auto fvs = redic.hgetall(key_1);
EXPECT_TRUE(fvs.empty());
fvs = redic.hgetall(key_2);

cout << "Get key [b]" << flush;
for (auto fv: fvs)
{
string value_1 = "1", value_2 = "2";
cout << " " << fvField(fv) << ":" << fvValue(fv) << flush;
if (fvField(fv) == "field_1")
{
EXPECT_EQ(fvValue(fv), value_1);
}
if (fvField(fv) == "field_2")
{
EXPECT_EQ(fvValue(fv), value_2);
}
}
cout << endl;

cout << "- Step 6. DEL and GET_TABLE_CONTENT" << endl;
cout << "Delete key [b]" << endl;
redic.del(key_2);
fvs = redic.hgetall(key_2);

EXPECT_TRUE(fvs.empty());

cout << "Done." << endl;
}

TEST(DBConnector, test)
{
thread *producerThreads[NUMBER_OF_THREADS];
Expand Down