This is an automated email from the ASF dual-hosted git repository.
alexey 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 34bf62310 [test_util] more accurate status codes for WaitForBind()
34bf62310 is described below
commit 34bf62310b0631ae3ee39ccce729fe83103de9cc
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Thu Aug 3 15:22:15 2023 -0700
[test_util] more accurate status codes for WaitForBind()
When troubleshooting failed test scenarios involving the mini_ranger and
the mini_ranger_kms test wrappers, I found it's hard to decipher error
messages when the underlying memory hogs were slow to start within the
specified timeout of 90 seconds.
This patch addresses the issue, so in case of timeout the WaitForBind()
utility returns Status::TimedOut() with comprehensive error message.
I also took the liberty of making the timeout tracking more precise and
fixing minor code style issues in the related code.
Change-Id: Ic4ed06540d3f4d86bb61f96ffb36ffc671e1c485
Reviewed-on: http://gerrit.cloudera.org:8080/20313
Reviewed-by: Alexey Serbin <ale...@apache.org>
Tested-by: Alexey Serbin <ale...@apache.org>
---
src/kudu/ranger-kms/mini_ranger_kms.cc | 7 ++++---
src/kudu/ranger/mini_ranger.cc | 7 +++----
src/kudu/util/test_util.cc | 34 +++++++++++++++++++++++-----------
3 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/src/kudu/ranger-kms/mini_ranger_kms.cc
b/src/kudu/ranger-kms/mini_ranger_kms.cc
index 7f78005d8..a373fccda 100644
--- a/src/kudu/ranger-kms/mini_ranger_kms.cc
+++ b/src/kudu/ranger-kms/mini_ranger_kms.cc
@@ -21,14 +21,15 @@
#include <initializer_list>
#include <ostream>
#include <string>
+#include <type_traits>
#include <vector>
#include <glog/logging.h>
#include "kudu/gutil/strings/substitute.h"
-#include "kudu/postgres/mini_postgres.h"
+#include "kudu/postgres/mini_postgres.h" // IWYU pragma: keep
#include "kudu/ranger-kms/mini_ranger_kms_configs.h"
-#include "kudu/ranger/mini_ranger.h"
+#include "kudu/ranger/mini_ranger.h" // IWYU pragma: keep
#include "kudu/util/easy_json.h"
#include "kudu/util/env_util.h"
#include "kudu/util/faststring.h"
@@ -275,7 +276,7 @@ Status MiniRangerKMS::StartRangerKMS() {
RETURN_NOT_OK(WaitForTcpBind(process_->pid(),
&port_,
{ "0.0.0.0", "127.0.0.1", },
- MonoDelta::FromMilliseconds(90000)));
+ MonoDelta::FromSeconds(90)));
LOG(INFO) << "Ranger KMS bound to " << port_;
LOG(INFO) << "Ranger KMS URL: " << ranger_kms_url_;
}
diff --git a/src/kudu/ranger/mini_ranger.cc b/src/kudu/ranger/mini_ranger.cc
index e8ce7232c..3d9e39be4 100644
--- a/src/kudu/ranger/mini_ranger.cc
+++ b/src/kudu/ranger/mini_ranger.cc
@@ -20,6 +20,7 @@
#include <csignal>
#include <ostream>
#include <string>
+#include <type_traits>
#include <vector>
#include <glog/logging.h>
@@ -27,7 +28,7 @@
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/strings/join.h"
#include "kudu/gutil/strings/substitute.h"
-#include "kudu/postgres/mini_postgres.h"
+#include "kudu/postgres/mini_postgres.h" // IWYU pragma: keep
#include "kudu/ranger/mini_ranger_configs.h"
#include "kudu/ranger/ranger.pb.h"
#include "kudu/util/curl_util.h"
@@ -46,8 +47,6 @@ using std::string;
using std::vector;
using strings::Substitute;
-static constexpr int kRangerStartTimeoutMs = 90000;
-
namespace kudu {
namespace ranger {
@@ -236,7 +235,7 @@ Status MiniRanger::StartRanger() {
RETURN_NOT_OK(WaitForTcpBind(process_->pid(),
&port,
{ "0.0.0.0", "127.0.0.1", },
-
MonoDelta::FromMilliseconds(kRangerStartTimeoutMs)));
+ MonoDelta::FromSeconds(90)));
LOG(INFO) << "Ranger bound to " << port;
LOG(INFO) << "Ranger admin URL: " << ranger_admin_url_;
}
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index cd551bf1c..1be36160b 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -17,10 +17,11 @@
#include "kudu/util/test_util.h"
-#include <limits.h>
#include <unistd.h>
+#include <algorithm>
#include <cerrno>
+#include <climits>
#include <cstdlib>
#include <limits>
#include <map>
@@ -452,7 +453,8 @@ int CountOpenFds(Env* env, const string& path_pattern) {
namespace {
const vector<string> kWildcard = { "0.0.0.0" };
-Status WaitForBind(pid_t pid, uint16_t* port,
+Status WaitForBind(pid_t pid,
+ uint16_t* port,
const vector<string>& addresses,
const char* kind,
MonoDelta timeout) {
@@ -494,10 +496,10 @@ Status WaitForBind(pid_t pid, uint16_t* port,
const auto& addresses_to_check = addresses.empty() ? kWildcard : addresses;
for (int64_t i = 1; ; ++i) {
for (const auto& addr : addresses_to_check) {
- string addr_pattern = Substitute("n$0:", addr == "0.0.0.0" ? "*" : addr);
+ const string addr_pattern = Substitute("n$0:", addr == "0.0.0.0" ? "*" :
addr);
string lsof_out;
int32_t p = -1;
- Status s = Subprocess::Call(cmd, "", &lsof_out).AndThen([&] () {
+ const auto s = Subprocess::Call(cmd, "", &lsof_out).AndThen([&] () {
StripTrailingNewline(&lsof_out);
vector<string> lines = strings::Split(lsof_out, "\n");
for (int index = 2; index < lines.size(); index += 2) {
@@ -506,18 +508,21 @@ Status WaitForBind(pid_t pid, uint16_t* port,
!cur_line.contains("->")) {
cur_line.remove_prefix(addr_pattern.size());
if (!safe_strto32(cur_line.data(), cur_line.size(), &p)) {
- return Status::RuntimeError("unexpected lsof output", lsof_out);
+ return Status::RuntimeError(Substitute(
+ "could not parse port number in string '$0' from lsof
output",
+ string(cur_line.data(), cur_line.size())), lsof_out);
}
return Status::OK();
}
}
- return Status::RuntimeError("unexpected lsof output", lsof_out);
+ return Status::NotFound(
+ "could not find pattern of a bound port in lsof output", lsof_out);
});
if (s.ok()) {
- CHECK(p > 0 && p < std::numeric_limits<uint16_t>::max())
+ CHECK(p > 0 && p <= std::numeric_limits<uint16_t>::max())
<< "parsed invalid port: " << p;
VLOG(1) << "Determined bound port: " << p;
*port = static_cast<uint16_t>(p);
@@ -525,10 +530,15 @@ Status WaitForBind(pid_t pid, uint16_t* port,
return Status::OK();
}
if (deadline < MonoTime::Now()) {
- return s;
+ return Status::TimedOut(Substitute(
+ "process with PID $0 is not yet bound to any port at the specified
"
+ "addresses; last attempt running lsof returned '$1'",
+ pid, s.ToString()));
}
}
- SleepFor(MonoDelta::FromMilliseconds(i * 10));
+ auto time_left_ms = std::max<int64_t>(
+ (deadline - MonoTime::Now()).ToMilliseconds(), 0);
+ SleepFor(MonoDelta::FromMilliseconds(std::min<int64_t>(i * 10,
time_left_ms)));
}
// Should not reach here.
@@ -574,7 +584,7 @@ Status WaitForBindAtPort(const vector<string>& addresses,
"n$0:$1", addr == "0.0.0.0" ? "*" : addr, port);
for (const auto& l : lines) {
if (l.empty()) {
- return Status::RuntimeError("unexpected lsof output", lsof_out);
+ return Status::RuntimeError("empty line in lsof output", lsof_out);
}
if (l[0] == 'p' || l[0] == 'f') {
continue;
@@ -592,7 +602,9 @@ Status WaitForBindAtPort(const vector<string>& addresses,
if (deadline < MonoTime::Now()) {
break;
}
- SleepFor(MonoDelta::FromMilliseconds(i * 10));
+ auto time_left_ms = std::max<int64_t>(
+ (deadline - MonoTime::Now()).ToMilliseconds(), 0);
+ SleepFor(MonoDelta::FromMilliseconds(std::min<int64_t>(i * 10,
time_left_ms)));
}
return Status::TimedOut(
