acelyc111 commented on code in PR #1091:
URL: https://github.com/apache/incubator-pegasus/pull/1091#discussion_r947421269
##########
src/shell/commands/data_operations.cpp:
##########
@@ -2397,6 +2398,15 @@ bool count_data(command_executor *e, shell_context *sc,
arguments args)
options.no_value = false;
else
options.no_value = true;
+
+ if (diff_hash_key || stat_size || value_filter_type !=
pegasus::pegasus_client::FT_NO_FILTER ||
+ sort_key_filter_type == pegasus::pegasus_client::FT_MATCH_EXACT) {
+ options.only_return_count = false;
Review Comment:
The jugement is complex, add comments to explain why.
##########
src/test/function_test/test_scan.cpp:
##########
@@ -168,6 +168,34 @@ class scan : public testing::Test
}
};
+TEST_F(scan, OVERALL_COUNT_ONLY)
+{
+ ddebug("TEST OVERALL_SCAN_COUNT_ONLY...");
+ pegasus_client::scan_options options;
+ options.only_return_count = true;
+ std::vector<pegasus_client::pegasus_scanner *> scanners;
+ int ret = client->get_unordered_scanners(3, options, scanners);
+ ASSERT_EQ(0, ret) << "Error occurred when getting scanner. error="
+ << client->get_error_string(ret);
+ ASSERT_LE(scanners.size(), 3);
+
+ std::string hash_key;
+ std::string sort_key;
+ std::string value;
+ int32_t data_counts = 0;
+ for (auto scanner : scanners) {
+ ASSERT_NE(nullptr, scanner);
+ int32_t kv_count;
+ while (PERR_OK == (ret = (scanner->next(hash_key, sort_key, value,
nullptr, &kv_count)))) {
+ data_counts += kv_count;
+ }
+ ASSERT_EQ(PERR_SCAN_COMPLETE, ret) << "Error occurred when scan.
error="
+ << client->get_error_string(ret);
+ delete scanner;
+ }
+ ASSERT_EQ(10990, data_counts);
Review Comment:
where dose 10990 come from? it would be better to use a variable from
previous variables, do not use a magic number.
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -261,6 +278,18 @@ void
pegasus_client_impl::pegasus_scanner_impl::_on_scan_response(::dsn::error_c
_kvs = std::move(response.kvs);
_p = -1;
_context = response.context_id;
+ // 1. kv_count exist on response means server is newer version
(added counting size only
+ // implementation)
+ // 1> kv_count == -1 indicates response still have key and value
data
+ // 2> kv_count > -1 indicates response only have kv size count,
but not key && value
+ // 2. kv_count is not existed means server is older version
+ if (response.__isset.kv_count) {
+ if (response.kv_count != -1) {
+ _only_calculate_count = true;
Review Comment:
Request is sent from client, we can init _only_calculate_count when send
request, right?
##########
idl/rrdb.thrift:
##########
@@ -279,6 +279,7 @@ struct get_scanner_request
11:optional bool validate_partition_hash;
12:optional bool return_expire_ts;
13:optional bool full_scan; // true means client want to build 'full scan'
context with the server side, false otherwise
+ 14:optional bool only_return_count;
Review Comment:
set the default value to false?
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -1312,6 +1319,7 @@ void pegasus_server_impl::on_scan(scan_rpc rpc)
resp.app_id = _gpid.get_app_id();
resp.partition_index = _gpid.get_partition_index();
resp.server = _primary_address;
+ bool only_return_count = false;
Review Comment:
Define it in `if (context) {...}` scope is enough.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
