This is an automated email from the ASF dual-hosted git repository.
zhangyifan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 7618d34e4 [tserver] add TabletServerTest.InvalidScanRequestNoGreaterKey
7618d34e4 is described below
commit 7618d34e4c60ee1c14f5cb734ea0c4cfa581dfde
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Tue Jul 19 20:28:34 2022 -0700
[tserver] add TabletServerTest.InvalidScanRequestNoGreaterKey
This patch adds a new test scenario into TabletServerTest to cover
the case when client sends INT32_MAX value in last_primary_key with
ScanRequestPB::new_scan_request field. That's to verify that tablet
server sends back an error with TabletServerErrorPB::INVALID_SCAN_SPEC
code.
I also updated log message in TabletServiceImpl::Scan() to output
information about scanner identifier upon an error setting up scanner
for a scan request. In addition, I took the liberty to update the code
around, getting rid of unused variables and making it more style guide
compliant.
Change-Id: I0992997e82114142e7f93c7898491b08208e1823
Reviewed-on: http://gerrit.cloudera.org:8080/18756
Tested-by: Kudu Jenkins
Reviewed-by: Yifan Zhang <chinazhangyi...@163.com>
---
src/kudu/tserver/tablet_server-test.cc | 20 ++++++++++++++++++++
src/kudu/tserver/tablet_service.cc | 23 +++++++++++------------
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/src/kudu/tserver/tablet_server-test.cc
b/src/kudu/tserver/tablet_server-test.cc
index e08b827c7..0adf63cd7 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -3483,6 +3483,26 @@ TEST_F(TabletServerTest,
TestInvalidScanRequest_UnknownOrderMode) {
"Unknown order mode specified"));
}
+TEST_F(TabletServerTest, InvalidScanRequestNoGreaterKey) {
+ const int32_t key_val = INT32_MAX;
+ Arena arena(64);
+ EncodedKeyBuilder ekb(&schema_, &arena);
+ ekb.AddColumnKey(&key_val);
+ EncodedKey* key_encoded = ekb.BuildEncodedKey();
+
+ ScanRequestPB req;
+ NewScanRequestPB* scan = req.mutable_new_scan_request();
+ scan->set_tablet_id(kTabletId);
+ scan->set_order_mode(OrderMode::ORDERED);
+ scan->set_read_mode(ReadMode::READ_AT_SNAPSHOT);
+ scan->set_last_primary_key(key_encoded->encoded_key().ToString());
+ ASSERT_OK(SchemaToColumnPBs(schema_, scan->mutable_projected_columns()));
+ req.set_call_seq_id(0);
+ NO_FATALS(VerifyScanRequestFailure(req,
+ TabletServerErrorPB::INVALID_SCAN_SPEC,
+ "No lexicographically greater key
exists"));
+}
+
// Test that passing a projection with Column IDs throws an exception.
// Column IDs are assigned to the user request schema on the tablet server
// based on the latest schema.
diff --git a/src/kudu/tserver/tablet_service.cc
b/src/kudu/tserver/tablet_service.cc
index 6e1ae19b6..f1704b001 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -2191,8 +2191,7 @@ void TabletServiceImpl::Scan(const ScanRequestPB* req,
}
}
- size_t batch_size_bytes = GetMaxBatchSizeBytesHint(req);
- ScanResultCopier collector(batch_size_bytes);
+ ScanResultCopier collector(GetMaxBatchSizeBytesHint(req));
bool has_more_results = false;
TabletServerErrorPB::Code error_code = TabletServerErrorPB::UNKNOWN_ERROR;
@@ -2950,8 +2949,9 @@ Status
TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
TRACE("Iterator init: $0", s.ToString());
if (PREDICT_FALSE(!s.ok())) {
- LOG(WARNING) << Substitute("Error setting up scanner with request: $0: $1",
- s.ToString(), SecureShortDebugString(*req));
+ LOG(WARNING) << Substitute(
+ "error setting up scanner $0 with request: $1: $2",
+ scanner->id(), s.ToString(), SecureShortDebugString(*req));
// If the replica has been stopped, e.g. due to disk failure, return
// TABLET_FAILED so the scan can be handled appropriately (fail over to
// another tablet server if fault-tolerant).
@@ -3005,8 +3005,7 @@ Status
TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
VLOG(1) << "Started scanner " << scanner->id() << ": " <<
scanner->iter()->ToString();
- size_t batch_size_bytes = GetMaxBatchSizeBytesHint(req);
- if (batch_size_bytes > 0) {
+ if (GetMaxBatchSizeBytesHint(req) > 0) {
TRACE("Continuing scan request");
// TODO(wdberkeley): Instead of copying the pb, instead split
// HandleContinueScanRequest and call the second half directly. Once that's
@@ -3016,13 +3015,13 @@ Status
TabletServiceImpl::HandleNewScanRequest(TabletReplica* replica,
ScanRequestPB continue_req(*req);
continue_req.set_scanner_id(scanner->id());
scanner_lock.Unlock();
- RETURN_NOT_OK(HandleContinueScanRequest(&continue_req, rpc_context,
result_collector,
- has_more_results, error_code));
- } else {
- // Increment the scanner call sequence ID. HandleContinueScanRequest
handles
- // this in the non-empty scan case.
- scanner->IncrementCallSeqId();
+ return HandleContinueScanRequest(
+ &continue_req, rpc_context, result_collector, has_more_results,
error_code);
}
+
+ // Increment the scanner call sequence ID. HandleContinueScanRequest handles
+ // this in the non-empty scan case.
+ scanner->IncrementCallSeqId();
return Status::OK();
}
