From 93ee96f7e39575547e3e548ef96d027726f105f1 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 8 Jul 2022 06:44:44 +0000 Subject: [PATCH 1/9] Improve hmset with pipe --- common/dbconnector.cpp | 31 +++++++------------------------ common/redispipeline.h | 1 + 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 9c725174f..0d316f85a 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -11,6 +11,7 @@ #include "common/dbconnector.h" #include "common/redisreply.h" #include "common/redisapi.h" +#include "common/redispipeline.h" #include "common/pubsub.h" using json = nlohmann::json; @@ -880,33 +881,15 @@ void DBConnector::hmset(const std::unordered_map(item)] = std::get<1>(item); - } - - j[kvp.first] = o; + RedisCommand hmset; + hmset.formatHMSET(hash.first, hash.second.begin(), hash.second.end()); + pipe.push(hmset, REDIS_REPLY_INTEGER); } - std::string strJson = j.dump(); - - lazyLoadRedisScriptFile(this, "redis_multi.lua", m_shaRedisMulti); - RedisCommand command; - command.format( - "EVALSHA %s 1 %s %s", - m_shaRedisMulti.c_str(), - strJson.c_str(), - "mhset"); - - RedisReply r(this, command, REDIS_REPLY_NIL); + pipe.flush(); } void DBConnector::del(const std::vector& keys) diff --git a/common/redispipeline.h b/common/redispipeline.h index 29d12df6c..1a77cf92a 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include "redisreply.h" #include "rediscommand.h" From 2db5dbf3ac9f189002e056d6b2f5cc5c00aec9f0 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 13 Jul 2022 06:52:58 +0000 Subject: [PATCH 2/9] Improve hmset and hdel performance with RedisPipeline --- common/configdb.cpp | 12 ++++++------ common/dbconnector.cpp | 27 +++++++++------------------ common/dbconnector.h | 6 +++--- common/redis_multi.lua | 20 -------------------- common/rediscommand.cpp | 10 ++++++++-- common/rediscommand.h | 11 +++++++---- common/table.cpp | 4 ++-- 7 files changed, 35 insertions(+), 55 deletions(-) delete mode 100644 common/redis_multi.lua diff --git a/common/configdb.cpp b/common/configdb.cpp index 6d8747bf3..d1aab140f 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -365,9 +365,9 @@ void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::str { auto original = get_entry(table, key); - RedisCommand shmset; - shmset.formatHMSET(_hash, data.begin(), data.end()); - pipe.enqueue(shmset, REDIS_REPLY_STATUS); + RedisCommand shset; + shset.formatHSET(_hash, data.begin(), data.end()); + pipe.enqueue(shset, REDIS_REPLY_INTEGER); for (auto& it: original) { @@ -403,9 +403,9 @@ void ConfigDBPipeConnector_Native::_mod_entry(RedisTransactioner& pipe, string t } else { - RedisCommand shmset; - shmset.formatHMSET(_hash, data.begin(), data.end()); - pipe.enqueue(shmset, REDIS_REPLY_STATUS); + RedisCommand shset; + shset.formatHSET(_hash, data.begin(), data.end()); + pipe.enqueue(shset, REDIS_REPLY_INTEGER); } } // Write multiple tables into config db. diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 0d316f85a..881a58b5e 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -884,9 +884,9 @@ void DBConnector::hmset(const std::unordered_map& keys) { SWSS_LOG_ENTER(); - json j = json::array(); - - for (const auto& key: keys) + RedisPipeline pipe(this); + for (auto& key : keys) { - j.push_back(key); + RedisCommand del; + del.formatDEL(key); + pipe.push(del, REDIS_REPLY_INTEGER); } - std::string strJson = j.dump(); - - lazyLoadRedisScriptFile(this, "redis_multi.lua", m_shaRedisMulti); - RedisCommand command; - command.format( - "EVALSHA %s 1 %s %s", - m_shaRedisMulti.c_str(), - strJson.c_str(), - "mdel"); - - RedisReply r(this, command, REDIS_REPLY_NIL); + pipe.flush(); } diff --git a/common/dbconnector.h b/common/dbconnector.h index 4a36cd3e8..47934ff31 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -284,9 +284,9 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) template void DBConnector::hmset(const std::string &key, InputIterator start, InputIterator stop) { - RedisCommand shmset; - shmset.formatHMSET(key, start, stop); - RedisReply r(this, shmset, REDIS_REPLY_STATUS); + RedisCommand shset; + shset.formatHSET(key, start, stop); + RedisReply r(this, shset, REDIS_REPLY_INTEGER); } } diff --git a/common/redis_multi.lua b/common/redis_multi.lua deleted file mode 100644 index 08c477bc4..000000000 --- a/common/redis_multi.lua +++ /dev/null @@ -1,20 +0,0 @@ -local op = ARGV[1] -local jj = cjson.decode(KEYS[1]) - -if op == "mhset" then - - for keyname,o in pairs(jj) do - for k,v in pairs(o) do - redis.call('HSET', keyname, k, v) - end - end - -elseif op == "mdel" then - - for idx,keyname in ipairs(jj) do - redis.call('DEL', keyname) - end - -else - error("unsupported operation command: " .. op .. ", FIXME") -end diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp index a24f3a6b8..36212532c 100644 --- a/common/rediscommand.cpp +++ b/common/rediscommand.cpp @@ -60,10 +60,10 @@ void RedisCommand::format(const vector &commands) } /* Format HMSET key multiple field value command */ -void RedisCommand::formatHMSET(const std::string &key, +void RedisCommand::formatHSET(const std::string &key, const std::vector &values) { - formatHMSET(key, values.begin(), values.end()); + formatHSET(key, values.begin(), values.end()); } /* Format HSET key field value command */ @@ -110,6 +110,12 @@ void RedisCommand::formatTTL(const std::string& key) return format("TTL %s", key.c_str()); } +/* Format DEL key command */ +void RedisCommand::formatDEL(const std::string& key) +{ + return format("DEL %s", key.c_str()); +} + const char *RedisCommand::c_str() const { return temp; diff --git a/common/rediscommand.h b/common/rediscommand.h index 5ea4c6122..6082b6312 100644 --- a/common/rediscommand.h +++ b/common/rediscommand.h @@ -31,11 +31,11 @@ class RedisCommand { #ifndef SWIG __attribute__((deprecated)) #endif - void formatHMSET(const std::string &key, + void formatHSET(const std::string &key, const std::vector &values); template - void formatHMSET(const std::string &key, + void formatHSET(const std::string &key, InputIterator start, InputIterator stop); /* Format HSET key field value command */ @@ -57,6 +57,9 @@ class RedisCommand { /* Format TTL key command */ void formatTTL(const std::string& key); + /* Format DEL key command */ + void formatDEL(const std::string& key); + const char *c_str() const; size_t length() const; @@ -66,12 +69,12 @@ class RedisCommand { }; template -void RedisCommand::formatHMSET(const std::string &key, +void RedisCommand::formatHSET(const std::string &key, InputIterator start, InputIterator stop) { if (start == stop) throw std::invalid_argument("empty values"); - const char* cmd = "HMSET"; + const char* cmd = "HSET"; std::vector args = { cmd, key.c_str() }; diff --git a/common/table.cpp b/common/table.cpp index cc2234d07..a048cf052 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -140,8 +140,8 @@ void Table::set(const string &key, const vector &values, RedisCommand cmd; - cmd.formatHMSET(getKeyName(key), values.begin(), values.end()); - m_pipe->push(cmd, REDIS_REPLY_STATUS); + cmd.formatHSET(getKeyName(key), values.begin(), values.end()); + m_pipe->push(cmd, REDIS_REPLY_INTEGER); if (ttl != DEFAULT_DB_TTL) { From d5dd8021d3326b05d649cf5e450f2ee58355242c Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 13 Jul 2022 07:07:58 +0000 Subject: [PATCH 3/9] Improve code --- common/Makefile.am | 1 - common/dbconnector.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 17cff9ab8..b621adff0 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -7,7 +7,6 @@ EXTRA_DIST = \ consumer_table_pops.lua \ producer_state_table_apply_view.lua \ table_dump.lua \ - redis_multi.lua \ portcounter.lua \ fdb_flush.lua \ fdb_flush.v2.lua diff --git a/common/dbconnector.h b/common/dbconnector.h index 47934ff31..88e6153fe 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -251,8 +251,6 @@ class DBConnector : public RedisContext int m_dbId; std::string m_dbName; std::string m_namespace; - - std::string m_shaRedisMulti; }; template From 63cd5b6cea0b0b141c942dbd36054faa44161334 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 13 Jul 2022 08:08:49 +0000 Subject: [PATCH 4/9] Improve UT coverage --- tests/redis_ut.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index c3e590d7a..16384c0d7 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -477,6 +477,58 @@ TEST(DBConnector, RedisClient) cout << "Done." << endl; } +TEST(DBConnector, HmsetAndDel) +{ + DBConnector db("TEST_DB", 0, true); + clearDB(); + + unordered_map>> multiHash; + vector values; + values.push_back(make_pair("field_1", "1")); + values.push_back(make_pair("field_2", "2")); + + vector keys; + for (int idx =0; idx<10; idx++) + { + string key = "hash_key_" + to_string(idx); + multiHash[key] = values; + keys.push_back(key); + } + + // set multiple hash with hmset + db.hmset(multiHash); + + // check all key exist + for (auto& key : keys) + { + auto fvs = db.hgetall(key); + EXPECT_EQ(fvs.size(), 2); + + for (auto fv: fvs) + { + string value_1 = "1", value_2 = "2"; + if (fvField(fv) == "field_1") + { + EXPECT_EQ(fvValue(fv), value_1); + } + if (fvField(fv) == "field_2") + { + EXPECT_EQ(fvValue(fv), value_2); + } + } + } + + // delete multiple hash with del + db.del(keys); + + // check all key deleted + for (auto& key : keys) + { + auto fvs = db.hgetall(key); + EXPECT_EQ(fvs.size(), 0); + } +} + TEST(DBConnector, test) { thread *producerThreads[NUMBER_OF_THREADS]; From 09e8ca9fe61d1775ec143d347ced783f6f6905d5 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 14 Jul 2022 07:25:15 +0000 Subject: [PATCH 5/9] Improve code --- common/dbconnector.cpp | 10 +++------- common/redispipeline.h | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 881a58b5e..ada65f8d7 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -10,7 +10,7 @@ #include "common/dbconnector.h" #include "common/redisreply.h" -#include "common/redisapi.h" +//#include "common/redisapi.h" #include "common/redispipeline.h" #include "common/pubsub.h" @@ -884,9 +884,7 @@ void DBConnector::hmset(const std::unordered_map& keys) RedisPipeline pipe(this); for (auto& key : keys) { - RedisCommand del; - del.formatDEL(key); - pipe.push(del, REDIS_REPLY_INTEGER); + pipe.pushDel(key); } pipe.flush(); diff --git a/common/redispipeline.h b/common/redispipeline.h index 1a77cf92a..0be92422b 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -62,6 +62,26 @@ class RedisPipeline { return r.release(); } + redisReply *pushHset(const std::string& key, const std::map &values) + { + return pushHset(key, values.begin(), values.end()); + } + + template + redisReply *pushHset(const std::string& key, const InputIterator& begin, const InputIterator& end) + { + RedisCommand hset; + hset.formatHSET(key, begin, end); + return push(hset, REDIS_REPLY_INTEGER); + } + + redisReply *pushDel(const std::string& key) + { + RedisCommand del; + del.formatDEL(key); + return push(del, REDIS_REPLY_INTEGER); + } + std::string loadRedisScript(const std::string& script) { RedisCommand loadcmd; From 78ea0af7cea5d9ea4a2d128e3769f3db27c74826 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 14 Jul 2022 08:56:01 +0000 Subject: [PATCH 6/9] Improve method name --- common/dbconnector.cpp | 4 ++-- common/redispipeline.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index ada65f8d7..e8a173856 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -884,7 +884,7 @@ void DBConnector::hmset(const std::unordered_map& keys) RedisPipeline pipe(this); for (auto& key : keys) { - pipe.pushDel(key); + pipe.pushDEL(key); } pipe.flush(); diff --git a/common/redispipeline.h b/common/redispipeline.h index 0be92422b..843862605 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -62,20 +62,20 @@ class RedisPipeline { return r.release(); } - redisReply *pushHset(const std::string& key, const std::map &values) + redisReply *pushHSET(const std::string& key, const std::map &values) { - return pushHset(key, values.begin(), values.end()); + return pushHSET(key, values.begin(), values.end()); } template - redisReply *pushHset(const std::string& key, const InputIterator& begin, const InputIterator& end) + redisReply *pushHSET(const std::string& key, const InputIterator& begin, const InputIterator& end) { RedisCommand hset; hset.formatHSET(key, begin, end); return push(hset, REDIS_REPLY_INTEGER); } - redisReply *pushDel(const std::string& key) + redisReply *pushDEL(const std::string& key) { RedisCommand del; del.formatDEL(key); From 5f2bf55b5ea7517ad91a734ba81e780d70a23d09 Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Fri, 15 Jul 2022 14:47:46 +0800 Subject: [PATCH 7/9] Remove unused reference. --- common/dbconnector.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index e8a173856..fe18a7778 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -10,7 +10,6 @@ #include "common/dbconnector.h" #include "common/redisreply.h" -//#include "common/redisapi.h" #include "common/redispipeline.h" #include "common/pubsub.h" From e0a8263a84c3fb2be568c7eeda7bcf4726e30633 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 15 Jul 2022 09:45:36 +0000 Subject: [PATCH 8/9] Improve code --- common/dbconnector.cpp | 8 ++++++-- common/rediscommand.cpp | 6 ++++++ common/rediscommand.h | 4 ++++ common/redispipeline.h | 20 -------------------- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index fe18a7778..ac17928a5 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -883,7 +883,9 @@ void DBConnector::hmset(const std::unordered_map& keys) RedisPipeline pipe(this); for (auto& key : keys) { - pipe.pushDEL(key); + RedisCommand del; + del.formatDEL(key); + pipe.push(del, REDIS_REPLY_INTEGER); } pipe.flush(); diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp index 36212532c..f423757f7 100644 --- a/common/rediscommand.cpp +++ b/common/rediscommand.cpp @@ -66,6 +66,12 @@ void RedisCommand::formatHSET(const std::string &key, formatHSET(key, values.begin(), values.end()); } +void RedisCommand::formatHSET(const std::string &key, + const std::map &values) +{ + formatHSET(key, values.begin(), values.end()); +} + /* Format HSET key field value command */ void RedisCommand::formatHSET(const std::string& key, const std::string& field, const std::string& value) diff --git a/common/rediscommand.h b/common/rediscommand.h index 6082b6312..74536c6ec 100644 --- a/common/rediscommand.h +++ b/common/rediscommand.h @@ -5,6 +5,7 @@ #include #include #include +#include namespace swss { @@ -34,6 +35,9 @@ class RedisCommand { void formatHSET(const std::string &key, const std::vector &values); + void formatHSET(const std::string &key, + const std::map &values); + template void formatHSET(const std::string &key, InputIterator start, InputIterator stop); diff --git a/common/redispipeline.h b/common/redispipeline.h index 843862605..1a77cf92a 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -62,26 +62,6 @@ class RedisPipeline { return r.release(); } - redisReply *pushHSET(const std::string& key, const std::map &values) - { - return pushHSET(key, values.begin(), values.end()); - } - - template - redisReply *pushHSET(const std::string& key, const InputIterator& begin, const InputIterator& end) - { - RedisCommand hset; - hset.formatHSET(key, begin, end); - return push(hset, REDIS_REPLY_INTEGER); - } - - redisReply *pushDEL(const std::string& key) - { - RedisCommand del; - del.formatDEL(key); - return push(del, REDIS_REPLY_INTEGER); - } - std::string loadRedisScript(const std::string& script) { RedisCommand loadcmd; From d937dcc6a0c73d22ed741b3e39f421da37e02e41 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 15 Jul 2022 09:52:19 +0000 Subject: [PATCH 9/9] Minor fix --- common/rediscommand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp index f423757f7..8156078e5 100644 --- a/common/rediscommand.cpp +++ b/common/rediscommand.cpp @@ -59,7 +59,7 @@ void RedisCommand::format(const vector &commands) formatArgv(static_cast(args.size()), args.data(), NULL); } -/* Format HMSET key multiple field value command */ +/* Format HSET key multiple field value command */ void RedisCommand::formatHSET(const std::string &key, const std::vector &values) {