Skip to content

Commit 4960226

Browse files
liuh-80itamar-talmon
liuh-80
authored and
itamar-talmon
committed
Improve hset and hdel performance with RedisPipeline. (sonic-net#647)
#### Why I did it 1.Change redis hmset command to hset because HMSET deprecated after redis 4.0.0: https://redis.io/commands/hmset/ 2.Improve DBConnector::hmset and DBConnector::del method performance. Currently swsscommon implement hmset and del by LUA script, however change to use RedisPipeline can improve performance by 20%: RedisPipeline hmset: 0.228743 LUA hmset: 0.260371 #### How I did it Change hmset and del to use RedisPipeline. #### How to verify it Pass all test case. #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 #### Description for the changelog #### Link to config_db schema for YANG module changes #### A picture of a cute animal (not mandatory but encouraged)
1 parent afab069 commit 4960226

10 files changed

+103
-81
lines changed

‎common/Makefile.am

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ EXTRA_DIST = \
77
consumer_table_pops.lua \
88
producer_state_table_apply_view.lua \
99
table_dump.lua \
10-
redis_multi.lua \
1110
portcounter.lua \
1211
fdb_flush.lua \
1312
fdb_flush.v2.lua

‎common/configdb.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,9 @@ void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::str
365365
{
366366
auto original = get_entry(table, key);
367367

368-
RedisCommand shmset;
369-
shmset.formatHMSET(_hash, data.begin(), data.end());
370-
pipe.enqueue(shmset, REDIS_REPLY_STATUS);
368+
RedisCommand shset;
369+
shset.formatHSET(_hash, data.begin(), data.end());
370+
pipe.enqueue(shset, REDIS_REPLY_INTEGER);
371371

372372
for (auto& it: original)
373373
{
@@ -403,9 +403,9 @@ void ConfigDBPipeConnector_Native::_mod_entry(RedisTransactioner& pipe, string t
403403
}
404404
else
405405
{
406-
RedisCommand shmset;
407-
shmset.formatHMSET(_hash, data.begin(), data.end());
408-
pipe.enqueue(shmset, REDIS_REPLY_STATUS);
406+
RedisCommand shset;
407+
shset.formatHSET(_hash, data.begin(), data.end());
408+
pipe.enqueue(shset, REDIS_REPLY_INTEGER);
409409
}
410410
}
411411
// Write multiple tables into config db.

‎common/dbconnector.cpp

+13-40
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
#include "common/dbconnector.h"
1212
#include "common/redisreply.h"
13-
#include "common/redisapi.h"
13+
#include "common/redispipeline.h"
1414
#include "common/pubsub.h"
1515

1616
using json = nlohmann::json;
@@ -880,55 +880,28 @@ void DBConnector::hmset(const std::unordered_map<std::string, std::vector<std::p
880880
{
881881
SWSS_LOG_ENTER();
882882

883-
// make sure this will be object (not null) when multi hash is empty
884-
json j = json::object();
885-
886-
// pack multi hash to json (takes bout 70 ms for 10k to construct)
887-
for (const auto& kvp: multiHash)
883+
RedisPipeline pipe(this);
884+
for (auto& hash : multiHash)
888885
{
889-
json o;
890-
891-
for (const auto &item: kvp.second)
892-
{
893-
o[std::get<0>(item)] = std::get<1>(item);
894-
}
895-
896-
j[kvp.first] = o;
886+
RedisCommand hset;
887+
hset.formatHSET(hash.first, hash.second.begin(), hash.second.end());
888+
pipe.push(hset, REDIS_REPLY_INTEGER);
897889
}
898890

899-
std::string strJson = j.dump();
900-
901-
lazyLoadRedisScriptFile(this, "redis_multi.lua", m_shaRedisMulti);
902-
RedisCommand command;
903-
command.format(
904-
"EVALSHA %s 1 %s %s",
905-
m_shaRedisMulti.c_str(),
906-
strJson.c_str(),
907-
"mhset");
908-
909-
RedisReply r(this, command, REDIS_REPLY_NIL);
891+
pipe.flush();
910892
}
911893

912894
void DBConnector::del(const std::vector<std::string>& keys)
913895
{
914896
SWSS_LOG_ENTER();
915897

916-
json j = json::array();
917-
918-
for (const auto& key: keys)
898+
RedisPipeline pipe(this);
899+
for (auto& key : keys)
919900
{
920-
j.push_back(key);
901+
RedisCommand del;
902+
del.formatDEL(key);
903+
pipe.push(del, REDIS_REPLY_INTEGER);
921904
}
922905

923-
std::string strJson = j.dump();
924-
925-
lazyLoadRedisScriptFile(this, "redis_multi.lua", m_shaRedisMulti);
926-
RedisCommand command;
927-
command.format(
928-
"EVALSHA %s 1 %s %s",
929-
m_shaRedisMulti.c_str(),
930-
strJson.c_str(),
931-
"mdel");
932-
933-
RedisReply r(this, command, REDIS_REPLY_NIL);
906+
pipe.flush();
934907
}

‎common/dbconnector.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,6 @@ class DBConnector : public RedisContext
251251
int m_dbId;
252252
std::string m_dbName;
253253
std::string m_namespace;
254-
255-
std::string m_shaRedisMulti;
256254
};
257255

258256
template <typename ReturnType>
@@ -284,9 +282,9 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result)
284282
template<typename InputIterator>
285283
void DBConnector::hmset(const std::string &key, InputIterator start, InputIterator stop)
286284
{
287-
RedisCommand shmset;
288-
shmset.formatHMSET(key, start, stop);
289-
RedisReply r(this, shmset, REDIS_REPLY_STATUS);
285+
RedisCommand shset;
286+
shset.formatHSET(key, start, stop);
287+
RedisReply r(this, shset, REDIS_REPLY_INTEGER);
290288
}
291289

292290
}

‎common/redis_multi.lua

-20
This file was deleted.

‎common/rediscommand.cpp

+15-3
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,17 @@ void RedisCommand::format(const vector<string> &commands)
5959
formatArgv(static_cast<int>(args.size()), args.data(), NULL);
6060
}
6161

62-
/* Format HMSET key multiple field value command */
63-
void RedisCommand::formatHMSET(const std::string &key,
62+
/* Format HSET key multiple field value command */
63+
void RedisCommand::formatHSET(const std::string &key,
6464
const std::vector<FieldValueTuple> &values)
6565
{
66-
formatHMSET(key, values.begin(), values.end());
66+
formatHSET(key, values.begin(), values.end());
67+
}
68+
69+
void RedisCommand::formatHSET(const std::string &key,
70+
const std::map<std::string, std::string> &values)
71+
{
72+
formatHSET(key, values.begin(), values.end());
6773
}
6874

6975
/* Format HSET key field value command */
@@ -110,6 +116,12 @@ void RedisCommand::formatTTL(const std::string& key)
110116
return format("TTL %s", key.c_str());
111117
}
112118

119+
/* Format DEL key command */
120+
void RedisCommand::formatDEL(const std::string& key)
121+
{
122+
return format("DEL %s", key.c_str());
123+
}
124+
113125
const char *RedisCommand::c_str() const
114126
{
115127
return temp;

‎common/rediscommand.h

+11-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <vector>
66
#include <string>
77
#include <stdexcept>
8+
#include <map>
89

910
namespace swss {
1011

@@ -31,11 +32,14 @@ class RedisCommand {
3132
#ifndef SWIG
3233
__attribute__((deprecated))
3334
#endif
34-
void formatHMSET(const std::string &key,
35+
void formatHSET(const std::string &key,
3536
const std::vector<FieldValueTuple> &values);
3637

38+
void formatHSET(const std::string &key,
39+
const std::map<std::string, std::string> &values);
40+
3741
template<typename InputIterator>
38-
void formatHMSET(const std::string &key,
42+
void formatHSET(const std::string &key,
3943
InputIterator start, InputIterator stop);
4044

4145
/* Format HSET key field value command */
@@ -57,6 +61,9 @@ class RedisCommand {
5761
/* Format TTL key command */
5862
void formatTTL(const std::string& key);
5963

64+
/* Format DEL key command */
65+
void formatDEL(const std::string& key);
66+
6067
const char *c_str() const;
6168

6269
size_t length() const;
@@ -66,12 +73,12 @@ class RedisCommand {
6673
};
6774

6875
template<typename InputIterator>
69-
void RedisCommand::formatHMSET(const std::string &key,
76+
void RedisCommand::formatHSET(const std::string &key,
7077
InputIterator start, InputIterator stop)
7178
{
7279
if (start == stop) throw std::invalid_argument("empty values");
7380

74-
const char* cmd = "HMSET";
81+
const char* cmd = "HSET";
7582

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

‎common/redispipeline.h

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include <string>
4+
#include <queue>
45
#include <functional>
56
#include "redisreply.h"
67
#include "rediscommand.h"

‎common/table.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ void Table::set(const string &key, const vector<FieldValueTuple> &values,
140140

141141
RedisCommand cmd;
142142

143-
cmd.formatHMSET(getKeyName(key), values.begin(), values.end());
144-
m_pipe->push(cmd, REDIS_REPLY_STATUS);
143+
cmd.formatHSET(getKeyName(key), values.begin(), values.end());
144+
m_pipe->push(cmd, REDIS_REPLY_INTEGER);
145145

146146
if (ttl != DEFAULT_DB_TTL)
147147
{

‎tests/redis_ut.cpp

+52
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,58 @@ TEST(DBConnector, RedisClient)
477477
cout << "Done." << endl;
478478
}
479479

480+
TEST(DBConnector, HmsetAndDel)
481+
{
482+
DBConnector db("TEST_DB", 0, true);
483+
clearDB();
484+
485+
unordered_map<string, vector<pair<string, string>>> multiHash;
486+
vector<FieldValueTuple> values;
487+
values.push_back(make_pair("field_1", "1"));
488+
values.push_back(make_pair("field_2", "2"));
489+
490+
vector<string> keys;
491+
for (int idx =0; idx<10; idx++)
492+
{
493+
string key = "hash_key_" + to_string(idx);
494+
multiHash[key] = values;
495+
keys.push_back(key);
496+
}
497+
498+
// set multiple hash with hmset
499+
db.hmset(multiHash);
500+
501+
// check all key exist
502+
for (auto& key : keys)
503+
{
504+
auto fvs = db.hgetall(key);
505+
EXPECT_EQ(fvs.size(), 2);
506+
507+
for (auto fv: fvs)
508+
{
509+
string value_1 = "1", value_2 = "2";
510+
if (fvField(fv) == "field_1")
511+
{
512+
EXPECT_EQ(fvValue(fv), value_1);
513+
}
514+
if (fvField(fv) == "field_2")
515+
{
516+
EXPECT_EQ(fvValue(fv), value_2);
517+
}
518+
}
519+
}
520+
521+
// delete multiple hash with del
522+
db.del(keys);
523+
524+
// check all key deleted
525+
for (auto& key : keys)
526+
{
527+
auto fvs = db.hgetall(key);
528+
EXPECT_EQ(fvs.size(), 0);
529+
}
530+
}
531+
480532
TEST(DBConnector, test)
481533
{
482534
thread *producerThreads[NUMBER_OF_THREADS];

0 commit comments

Comments
 (0)