This is an automated email from the ASF dual-hosted git repository.
achennaka 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 8bc0be58e [tserver] unify logging on rejected write requests
8bc0be58e is described below
commit 8bc0be58ef0a02f33d2ecd71cfe721fd16db1730
Author: Alexey Serbin <[email protected]>
AuthorDate: Mon Dec 4 11:47:36 2023 -0800
[tserver] unify logging on rejected write requests
This patch contain a minor clean up on the logging of rejected write
requests and other minor improvements, making the logs more useful
for post-mortem analysis and troubleshooting in some scenarios:
* unified the prefix for log messages on rejected write requests,
so it's now easier to search in the logs
* log about all rejected write requests using time-based throttling
wrapper KLOG_EVERY_N_SECS(.., 1)
* log about all rejected write requests using INFO severity unless
going over the --memory_limit_warn_threshold_percentage threshold
for memory pressure rejections; use WARNING severity for the latter
* changed TabletServerErrorPB from UNKNOWN_ERROR to THROTTLED when
rejecting a write request due to a throttling condition
* use static instances of Status objects to avoid allocating and
freeing memory for status messages upon rejections
Change-Id: I2ef97c54815b8d2e1895ce884befc7daf3d550f4
Reviewed-on: http://gerrit.cloudera.org:8080/20808
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
---
src/kudu/tserver/tablet_service.cc | 70 ++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/src/kudu/tserver/tablet_service.cc
b/src/kudu/tserver/tablet_service.cc
index a79fc955f..bc4bb7a4a 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -1576,15 +1576,29 @@ void TabletServiceImpl::Write(const WriteRequestPB* req,
// short-circuit further checking and reject the request immediately.
// Otherwise, we'll defer the checking to the prepare phase of the
// op after decoding the operations.
- LOG(WARNING) << Substitute("rejecting Write request from $0: no write
privileges",
- context->requestor_string());
- context->RespondRpcFailure(ErrorStatusPB::FATAL_UNAUTHORIZED,
- Status::NotAuthorized("not authorized to write"));
- return;
+ static const auto kStatus = Status::NotAuthorized("not authorized to
write");
+ const auto msg = Substitute(
+ "rejecting write request: no write privileges ($0)",
+ context->requestor_string());
+ KLOG_EVERY_N_SECS(INFO, 1) << msg << THROTTLE_MSG;
+ return context->RespondRpcFailure(ErrorStatusPB::FATAL_UNAUTHORIZED,
+ kStatus);
}
authz_context = WriteAuthorizationContext{ privileges,
/*requested_op_types=*/{} };
}
+ if (PREDICT_FALSE(!server_->clock()->SupportsExternalConsistencyMode(
+ req->external_consistency_mode()))) {
+ constexpr const char* const kMsg =
+ "rejecting write request: required consistency mode unsupported by
clock";
+ static const auto kStatus = Status::NotSupported(kMsg);
+ KLOG_EVERY_N_SECS(INFO, 1) << kMsg << THROTTLE_MSG;
+ return SetupErrorAndRespond(resp->mutable_error(),
+ kStatus,
+ TabletServerErrorPB::UNKNOWN_ERROR,
+ context);
+ }
+
shared_ptr<Tablet> tablet;
TabletServerErrorPB::Code error_code;
Status s = GetTabletRef(replica, &tablet, &error_code);
@@ -1593,40 +1607,36 @@ void TabletServiceImpl::Write(const WriteRequestPB* req,
return;
}
- uint64_t bytes = req->row_operations().rows().size() +
+ const uint64_t bytes = req->row_operations().rows().size() +
req->row_operations().indirect_data().size();
if (!tablet->ShouldThrottleAllow(bytes)) {
- SetupErrorAndRespond(resp->mutable_error(),
- Status::ServiceUnavailable("Rejecting Write request:
throttled"),
- TabletServerErrorPB::THROTTLED,
- context);
- return;
+ constexpr const char* const kMsg = "rejecting write request: throttled";
+ static const auto kStatus = Status::ServiceUnavailable(kMsg);
+ KLOG_EVERY_N_SECS(INFO, 1) << kMsg << THROTTLE_MSG;
+ return SetupErrorAndRespond(resp->mutable_error(),
+ kStatus,
+ TabletServerErrorPB::THROTTLED,
+ context);
}
// Check for memory pressure; don't bother doing any additional work if we've
// exceeded the limit.
double capacity_pct;
if (process_memory::SoftLimitExceeded(&capacity_pct)) {
+ constexpr const char* const kMsg =
+ "rejecting write request: soft memory limit exceeded";
+ static const auto kStatus = Status::ServiceUnavailable(kMsg);
tablet->metrics()->leader_memory_pressure_rejections->Increment();
- string msg = StringPrintf("Soft memory limit exceeded (at %.2f%% of
capacity)", capacity_pct);
+ string msg = StringPrintf("%s (at %.2f%% of capacity)", kMsg,
capacity_pct);
if (capacity_pct >= FLAGS_memory_limit_warn_threshold_percentage) {
- KLOG_EVERY_N_SECS(WARNING, 1) << "Rejecting Write request: " << msg <<
THROTTLE_MSG;
+ KLOG_EVERY_N_SECS(WARNING, 1) << msg << THROTTLE_MSG;
} else {
- KLOG_EVERY_N_SECS(INFO, 1) << "Rejecting Write request: " << msg <<
THROTTLE_MSG;
+ KLOG_EVERY_N_SECS(INFO, 1) << msg << THROTTLE_MSG;
}
- SetupErrorAndRespond(resp->mutable_error(),
Status::ServiceUnavailable(msg),
- TabletServerErrorPB::UNKNOWN_ERROR,
- context);
- return;
- }
-
- if
(!server_->clock()->SupportsExternalConsistencyMode(req->external_consistency_mode()))
{
- Status s = Status::NotSupported("The configured clock does not support the"
- " required consistency mode.");
- SetupErrorAndRespond(resp->mutable_error(), s,
- TabletServerErrorPB::UNKNOWN_ERROR,
- context);
- return;
+ return SetupErrorAndRespond(resp->mutable_error(),
+ kStatus,
+ TabletServerErrorPB::THROTTLED,
+ context);
}
// If the apply queue is overloaded, the write request might be rejected.
@@ -1642,9 +1652,11 @@ void TabletServiceImpl::Write(const WriteRequestPB* req,
// probability of an op to be rejected.
auto time_factor = queue_otime.ToMilliseconds() / overload_threshold_ms +
1;
if (!rng_.OneIn(time_factor * time_factor + 1)) {
- static const Status kStatus = Status::ServiceUnavailable(
- "op apply queue is overloaded");
+ constexpr const char* const kMsg =
+ "rejecting write request: apply queue overloaded";
+ static const auto kStatus = Status::ServiceUnavailable(kMsg);
num_op_apply_queue_rejections_->Increment();
+ KLOG_EVERY_N_SECS(INFO, 1) << kMsg << THROTTLE_MSG;
return SetupErrorAndRespond(resp->mutable_error(),
kStatus,
TabletServerErrorPB::THROTTLED,
