empiredan commented on code in PR #1091:
URL: https://github.com/apache/incubator-pegasus/pull/1091#discussion_r956602425
##########
src/shell/commands/data_operations.cpp:
##########
@@ -2397,6 +2397,17 @@ bool count_data(command_executor *e, shell_context *sc,
arguments args)
options.no_value = false;
else
options.no_value = true;
+
+ // cuz option only_return_count not return data(hashkey&sortkey&value) to
client
+ // all of this options need check data on client side
Review Comment:
I think maybe it's better to explain each of conditions here ?
```suggestion
// Decide whether real data should be returned to client. Once the real
data is
// decided not to be returned to client side: option `only_return_count`
will be
// used.
```
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -51,13 +51,38 @@
pegasus_client_impl::pegasus_scanner_impl::pegasus_scanner_impl(::dsn::apps::rrd
_options(options),
_splits_hash(std::move(hash)),
_p(-1),
+ _kv_count(-1),
_context(SCAN_CONTEXT_ID_COMPLETED),
_rpc_started(false),
_validate_partition_hash(validate_partition_hash),
- _full_scan(full_scan)
+ _full_scan(full_scan),
+ _type(async_scan_type::NORMAL)
{
}
+int pegasus_client_impl::pegasus_scanner_impl::next(int32_t &count,
internal_info *info)
+{
+ ::dsn::utils::notify_event op_completed;
+ int ret = -1;
+ auto callback = [&](int err,
+ std::string &&hash,
+ std::string &&sort,
+ std::string &&val,
+ internal_info &&ii,
+ uint32_t expire_ts_seconds,
+ int32_t kv_count) {
+ ret = err;
+ if (info) {
+ (*info) = std::move(ii);
+ }
Review Comment:
```suggestion
if (info != nullptr) {
*info = std::move(ii);
}
```
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -261,6 +297,16 @@ 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 mean (a && b):
+ // a> server is newer version (added counting size only
implementation)
+ // b> response only have kv size count, but not key && value
+ // 2. kv_count is not existed means (a || b):
+ // a> server is older version
+ // b> response still have key and value data
Review Comment:
I think there is no need to state "otherwise" here which is just the
negation of the existence of `kv_count`.
```suggestion
// If `kv_count` exists in response, then:
// 1) server side supports only counting size, and
// 2) `kvs` in response must be empty
```
--
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-unsubscr...@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pegasus.apache.org
For additional commands, e-mail: dev-h...@pegasus.apache.org
