You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "acelyc111 (via GitHub)" <gi...@apache.org> on 2023/02/28 14:22:25 UTC

[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1373: refactor(test): add bench type multi set/get

acelyc111 commented on code in PR #1373:
URL: https://github.com/apache/incubator-pegasus/pull/1373#discussion_r1120110793


##########
src/test/bench_test/statistics.cpp:
##########
@@ -26,7 +26,12 @@
 namespace pegasus {
 namespace test {
 std::unordered_map<operation_type, std::string, std::hash<unsigned char>> operation_type_string = {
-    {kUnknown, "unKnown"}, {kRead, "read"}, {kWrite, "write"}, {kDelete, "delete"}};
+    {kUnknown, "unKnown"},
+    {kRead, "read"},
+    {kWrite, "write"},
+    {kDelete, "delete"},
+    {kMultiSet, "multiset"},
+    {kMultiGet, "multiget"}};

Review Comment:
   ```suggestion
       {kMultiSet, "multiSet"},
       {kMultiGet, "multiGet"}};
   ```



##########
run.sh:
##########
@@ -1399,6 +1399,8 @@ function usage_bench()
     echo "                             fillrandom_pegasus       --pegasus write N random values with random keys list"
     echo "                             readrandom_pegasus       --pegasus read N times with random keys list"
     echo "                             deleterandom_pegasus     --pegasus delete N entries with random keys list"
+    echo "                             multisetrandom_pegasus   --pegasus write (N/100)*100 random values with N/100 random keys list"

Review Comment:
   What dose `(N/100)*100` mean?



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)
+{
+    uint64_t bytes = 0;
+    uint64_t found = 0;
+    int max_fetch_count = 100;
+    int max_fetch_size = 1000000;
+
+    for (int i = 0; i < FLAGS_benchmark_num / 100; i++) {
+
+        // generate hash key
+        std::string hashkey, sort_key, value;

Review Comment:
   `sort_key` is not used?



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)
+{
+    uint64_t bytes = 0;
+    uint64_t found = 0;
+    int max_fetch_count = 100;
+    int max_fetch_size = 1000000;
+
+    for (int i = 0; i < FLAGS_benchmark_num / 100; i++) {
+
+        // generate hash key
+        std::string hashkey, sort_key, value;
+        hashkey = generate_string(FLAGS_hashkey_size);
+        std::map<std::string, std::string> kvs;
+        std::set<std::string> sortkeys;
+
+        // generate sort key
+        // generate value for random to keep in peace with write
+        for (int j = 0; j < 100; j++) {
+            sortkeys.insert(generate_string(FLAGS_sortkey_size));
+            value = generate_string(FLAGS_value_size);
+        }
+
+        // read from pegasus
+        int try_count = 0;
+        while (true) {
+            try_count++;
+            int ret = _client->multi_get(
+                hashkey, sortkeys, kvs, max_fetch_count, max_fetch_size, FLAGS_pegasus_timeout_ms);
+            if (ret == ::pegasus::PERR_OK) {
+                found += 100;
+                bytes += (FLAGS_hashkey_size + FLAGS_sortkey_size + FLAGS_value_size) * 100;
+                break;
+            } else if (ret == ::pegasus::PERR_NOT_FOUND) {
+                break;
+            } else if (ret != ::pegasus::PERR_TIMEOUT || try_count > 3) {
+                fmt::print(stderr, "Get returned an error: {}\n", _client->get_error_string(ret));
+                dsn_exit(1);
+            } else {
+                fmt::print(stderr, "Get timeout, retry({})\n", try_count);
+            }
+        }
+
+        // count this operation
+        thread->stats.finished_ops(1, kRead);

Review Comment:
   Should be `kMultiGet`?



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)
+{
+    uint64_t bytes = 0;
+    uint64_t found = 0;
+    int max_fetch_count = 100;
+    int max_fetch_size = 1000000;
+
+    for (int i = 0; i < FLAGS_benchmark_num / 100; i++) {
+

Review Comment:
   remove the blank line.



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)

Review Comment:
   IMO, nothing is immutable, you can introduce more flags for the new tests.



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)
+{
+    uint64_t bytes = 0;
+    uint64_t found = 0;
+    int max_fetch_count = 100;
+    int max_fetch_size = 1000000;
+
+    for (int i = 0; i < FLAGS_benchmark_num / 100; i++) {
+
+        // generate hash key
+        std::string hashkey, sort_key, value;
+        hashkey = generate_string(FLAGS_hashkey_size);
+        std::map<std::string, std::string> kvs;
+        std::set<std::string> sortkeys;
+
+        // generate sort key
+        // generate value for random to keep in peace with write
+        for (int j = 0; j < 100; j++) {
+            sortkeys.insert(generate_string(FLAGS_sortkey_size));
+            value = generate_string(FLAGS_value_size);

Review Comment:
   why overwrite `value` repetitive?



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -152,6 +154,48 @@ void benchmark::write_random(thread_arg *thread)
     thread->stats.add_bytes(bytes);
 }
 
+void benchmark::multi_set_random(thread_arg *thread)
+{
+    // do write operation num times

Review Comment:
   what does `num` mean?



##########
src/test/bench_test/benchmark.cpp:
##########
@@ -191,6 +235,58 @@ void benchmark::read_random(thread_arg *thread)
     thread->stats.add_message(msg);
 }
 
+void benchmark::multi_get_random(thread_arg *thread)
+{
+    uint64_t bytes = 0;
+    uint64_t found = 0;
+    int max_fetch_count = 100;
+    int max_fetch_size = 1000000;
+
+    for (int i = 0; i < FLAGS_benchmark_num / 100; i++) {
+
+        // generate hash key
+        std::string hashkey, sort_key, value;
+        hashkey = generate_string(FLAGS_hashkey_size);
+        std::map<std::string, std::string> kvs;
+        std::set<std::string> sortkeys;
+
+        // generate sort key
+        // generate value for random to keep in peace with write
+        for (int j = 0; j < 100; j++) {
+            sortkeys.insert(generate_string(FLAGS_sortkey_size));
+            value = generate_string(FLAGS_value_size);
+        }
+
+        // read from pegasus
+        int try_count = 0;
+        while (true) {
+            try_count++;
+            int ret = _client->multi_get(
+                hashkey, sortkeys, kvs, max_fetch_count, max_fetch_size, FLAGS_pegasus_timeout_ms);
+            if (ret == ::pegasus::PERR_OK) {
+                found += 100;
+                bytes += (FLAGS_hashkey_size + FLAGS_sortkey_size + FLAGS_value_size) * 100;

Review Comment:
   The returned count and bytes depends on `kvs`, not the FLAGS_*, suppose the case when the database is empty or filled by different flags.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org