You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/08/04 06:55:33 UTC
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1091: feat: improve performance of count_data
acelyc111 commented on code in PR #1091:
URL: https://github.com/apache/incubator-pegasus/pull/1091#discussion_r937403751
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -55,7 +55,35 @@ pegasus_client_impl::pegasus_scanner_impl::pegasus_scanner_impl(::dsn::apps::rrd
_rpc_started(false),
_validate_partition_hash(validate_partition_hash),
_full_scan(full_scan)
+
Review Comment:
remove the blank line
##########
idl/rrdb.thrift:
##########
@@ -279,11 +279,15 @@ 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 filter_type value_filter_type;
+ 15:optional dsn.blob value_filter_pattern;
Review Comment:
Are the two parameters related to this patch?
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -162,13 +194,15 @@ void pegasus_client_impl::pegasus_scanner_impl::_async_next_internal()
}
// valid data got
- std::string hash_key, sort_key;
- pegasus_restore_key(_kvs[_p].key, hash_key, sort_key);
- std::string value(_kvs[_p].value.data(), _kvs[_p].value.length());
- uint32_t expire_ts_seconds = _kvs[_p].__isset.expire_ts_seconds
- ? static_cast<uint32_t>(_kvs[_p].expire_ts_seconds)
- : 0;
-
+ std::string hash_key, sort_key, value = "";
Review Comment:
```suggestion
std::string hash_key, sort_key, value;
```
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -162,13 +194,15 @@ void pegasus_client_impl::pegasus_scanner_impl::_async_next_internal()
}
// valid data got
- std::string hash_key, sort_key;
- pegasus_restore_key(_kvs[_p].key, hash_key, sort_key);
- std::string value(_kvs[_p].value.data(), _kvs[_p].value.length());
- uint32_t expire_ts_seconds = _kvs[_p].__isset.expire_ts_seconds
- ? static_cast<uint32_t>(_kvs[_p].expire_ts_seconds)
- : 0;
-
+ std::string hash_key, sort_key, value = "";
+ uint32_t expire_ts_seconds = 0;
+ if (_kv_count == -1) {
Review Comment:
it suggest to add some comment to explain what dose `_kv_count == -1` mean
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -1389,6 +1419,11 @@ void pegasus_server_impl::on_scan(scan_rpc rpc)
it->Next();
}
+ if (request.only_return_count) {
Review Comment:
you can use the flag in pegasus_scan_context, right?
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -55,7 +55,35 @@ pegasus_client_impl::pegasus_scanner_impl::pegasus_scanner_impl(::dsn::apps::rrd
_rpc_started(false),
_validate_partition_hash(validate_partition_hash),
_full_scan(full_scan)
+
+{
+}
+
+int pegasus_client_impl::pegasus_scanner_impl::next(std::string &hashkey,
Review Comment:
why not reuse the exist `pegasus_client_impl::pegasus_scanner_impl::next` ?
##########
idl/rrdb.thrift:
##########
@@ -279,11 +279,15 @@ 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 filter_type value_filter_type;
+ 15:optional dsn.blob value_filter_pattern;
+ 16:optional bool only_return_count = false;
}
struct scan_request
{
1:i64 context_id;
+ 2:optional bool only_return_count;
Review Comment:
the `get_scanner_request ` has a `only_return_count` field, why add another one to `scan_request`?
--
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