You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by aw...@apache.org on 2020/03/11 03:45:52 UTC
[kudu] branch master updated (1b3b26d -> 1cb4a0a)
This is an automated email from the ASF dual-hosted git repository.
awong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 1b3b26d KUDU-3070 skip open block manager
new 3f14af3 ranger: instantiate Ranger queue size metrics
new 39bd474 subprocess: maintain a thread for fork/exec
new 1cb4a0a ranger: fix the expected main class for the subprocess
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/ranger/ranger_client.cc | 8 +++++---
src/kudu/subprocess/server.cc | 20 +++++++++++++++++++-
src/kudu/subprocess/server.h | 7 +++++++
src/kudu/subprocess/subprocess_server-test.cc | 19 +++++++++++++++++++
4 files changed, 50 insertions(+), 4 deletions(-)
[kudu] 02/03: subprocess: maintain a thread for fork/exec
Posted by aw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 39bd47462d7c3d57f7207e9eebd4a8102484f568
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Tue Mar 10 15:23:58 2020 -0700
subprocess: maintain a thread for fork/exec
If SubprocessServer::Init() is called from a short-lived thread, because
it does a fork/exec, the subprocess will silently be reaped when the
thread exits.
Specifically, we saw this happening upon initializing the
CatalogManager, which happens in a short-lived thread, when starting up
the Ranger client. This surfaced as us getting an EOF from the
subprocess receiver thread.
Change-Id: I803b1613ef1a988df1da4c908c2c37e1fbbdcf81
Reviewed-on: http://gerrit.cloudera.org:8080/15398
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
src/kudu/subprocess/server.cc | 20 +++++++++++++++++++-
src/kudu/subprocess/server.h | 7 +++++++
src/kudu/subprocess/subprocess_server-test.cc | 19 +++++++++++++++++++
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/src/kudu/subprocess/server.cc b/src/kudu/subprocess/server.cc
index 3c7607b..6687e3b 100644
--- a/src/kudu/subprocess/server.cc
+++ b/src/kudu/subprocess/server.cc
@@ -100,9 +100,23 @@ SubprocessServer::~SubprocessServer() {
Shutdown();
}
+void SubprocessServer::StartSubprocessThread(const StdStatusCallback& cb) {
+ Status s = process_->Start();
+ cb(s);
+ if (PREDICT_TRUE(s.ok())) {
+ // If we successfully started the process, we should stay alive until we
+ // shut down.
+ closing_.Wait();
+ }
+}
+
Status SubprocessServer::Init() {
VLOG(2) << "Starting the subprocess";
- RETURN_NOT_OK_PREPEND(process_->Start(), "Failed to start subprocess");
+ Synchronizer sync;
+ auto cb = sync.AsStdStatusCallback();
+ RETURN_NOT_OK(Thread::Create("subprocess", "start", &SubprocessServer::StartSubprocessThread,
+ this, cb, &read_thread_));
+ RETURN_NOT_OK_PREPEND(sync.Wait(), "Failed to start subprocess");
// Start the message protocol.
CHECK(!message_protocol_);
@@ -172,6 +186,9 @@ void SubprocessServer::Shutdown() {
if (deadline_checker_) {
deadline_checker_->Join();
}
+ if (start_thread_) {
+ start_thread_->Join();
+ }
for (const auto& t : responder_threads_) {
t->Join();
}
@@ -197,6 +214,7 @@ void SubprocessServer::ReceiveMessagesThread() {
if (s.IsEndOfFile()) {
// The underlying pipe was closed. We're likely shutting down.
DCHECK_EQ(0, closing_.count());
+ LOG(INFO) << "Received an EOF from the subprocess";
return;
}
// TODO(awong): getting an error here indicates that this server and the
diff --git a/src/kudu/subprocess/server.h b/src/kudu/subprocess/server.h
index 512966b..3c33e5b 100644
--- a/src/kudu/subprocess/server.h
+++ b/src/kudu/subprocess/server.h
@@ -230,6 +230,8 @@ class SubprocessServer {
private:
FRIEND_TEST(SubprocessServerTest, TestCallsReturnWhenShuttingDown);
+ void StartSubprocessThread(const StdStatusCallback& cb);
+
// Stop the subprocess and stop processing messages.
void Shutdown();
@@ -274,6 +276,11 @@ class SubprocessServer {
// Protocol with which to send and receive bytes to and from 'process_'.
std::shared_ptr<SubprocessProtocol> message_protocol_;
+ // Thread that runs the subprocess. Since the subprocess is run via
+ // fork/exec, this thread must stay alive for the lifetime of the server.
+ // Otherwise, the OS may silently kill the spawned child process.
+ scoped_refptr<Thread> start_thread_;
+
// Pulls requests off the request queue and serializes them via the
// message protocol.
scoped_refptr<Thread> write_thread_;
diff --git a/src/kudu/subprocess/subprocess_server-test.cc b/src/kudu/subprocess/subprocess_server-test.cc
index d14293f..857d61f 100644
--- a/src/kudu/subprocess/subprocess_server-test.cc
+++ b/src/kudu/subprocess/subprocess_server-test.cc
@@ -33,6 +33,7 @@
#include "kudu/subprocess/subprocess.pb.h"
#include "kudu/util/env.h"
#include "kudu/util/metrics.h"
+#include "kudu/util/monotime.h"
#include "kudu/util/path_util.h"
#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/status.h"
@@ -260,6 +261,24 @@ TEST_F(SubprocessServerTest, TestCallsReturnWhenShuttingDown) {
ASSERT_FALSE(s.ok());
}
+// Some usage of a subprocess warrants calling Init() from a short-lived
+// thread. Let's ensure there's no funny business when that happens (e.g.
+// ensure the OS doesn't reap the underlying process when the parent thread
+// exits).
+TEST_F(SubprocessServerTest, TestInitFromThread) {
+ Status s;
+ thread t([&] {
+ s = ResetSubprocessServer();
+ });
+ t.join();
+ ASSERT_OK(s);
+ // Wait a bit to give time for the OS to wreak havoc (though it shouldn't).
+ SleepFor(MonoDelta::FromSeconds(3));
+ SubprocessRequestPB request = CreateEchoSubprocessRequestPB(kHello);
+ SubprocessResponsePB response;
+ ASSERT_OK(server_->Execute(&request, &response));
+}
+
} // namespace subprocess
} // namespace kudu
[kudu] 03/03: ranger: fix the expected main class for the subprocess
Posted by aw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 1cb4a0ae3e253fbab26a9c4664550c9ca473ca98
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Tue Mar 10 17:51:33 2020 -0700
ranger: fix the expected main class for the subprocess
Change-Id: I5a7662ed286bda3f6f7930daae3ed19e8428fcab
Reviewed-on: http://gerrit.cloudera.org:8080/15400
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/ranger/ranger_client.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index fe3a477..6878d81 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -121,7 +121,7 @@ static const char* kUnauthorizedAction = "Unauthorized action";
static const char* kDenyNonRangerTableTemplate = "Denying action on table with invalid name $0. "
"Use 'kudu table rename_table' to rename it to "
"a Ranger-compatible name.";
-const char* kMainClass = "org.apache.kudu.ranger.RangerSubprocessMain";
+const char* kMainClass = "org.apache.kudu.subprocess.ranger.RangerSubprocessMain";
#define HISTINIT(member, x) member = METRIC_##x.Instantiate(entity)
RangerSubprocessMetrics::RangerSubprocessMetrics(const scoped_refptr<MetricEntity>& entity) {
[kudu] 01/03: ranger: instantiate Ranger queue size metrics
Posted by aw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 3f14af36507fddddf8ba735af7c7ac56396d1ecb
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Tue Mar 10 17:29:08 2020 -0700
ranger: instantiate Ranger queue size metrics
Hit a segfault when trying to run an actual Ranger subprocess instead of
a mock.
Change-Id: I29ac97df4f5cac3d658ee7a619df6a4bece6b924
Reviewed-on: http://gerrit.cloudera.org:8080/15399
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/ranger/ranger_client.cc | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index c8ca24f..fe3a477 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -126,12 +126,14 @@ const char* kMainClass = "org.apache.kudu.ranger.RangerSubprocessMain";
#define HISTINIT(member, x) member = METRIC_##x.Instantiate(entity)
RangerSubprocessMetrics::RangerSubprocessMetrics(const scoped_refptr<MetricEntity>& entity) {
HISTINIT(sp_inbound_queue_length, ranger_subprocess_inbound_queue_length);
- HISTINIT(sp_outbound_queue_length, ranger_subprocess_outbound_queue_length);
HISTINIT(sp_inbound_queue_time_ms, ranger_subprocess_inbound_queue_time_ms);
+ HISTINIT(sp_outbound_queue_length, ranger_subprocess_outbound_queue_length);
HISTINIT(sp_outbound_queue_time_ms, ranger_subprocess_outbound_queue_time_ms);
HISTINIT(sp_execution_time_ms, ranger_subprocess_execution_time_ms);
- HISTINIT(server_outbound_queue_time_ms, ranger_server_outbound_queue_time_ms);
+ HISTINIT(server_inbound_queue_size_bytes, ranger_server_inbound_queue_size_bytes);
HISTINIT(server_inbound_queue_time_ms, ranger_server_inbound_queue_time_ms);
+ HISTINIT(server_outbound_queue_size_bytes, ranger_server_outbound_queue_size_bytes);
+ HISTINIT(server_outbound_queue_time_ms, ranger_server_outbound_queue_time_ms);
}
#undef HISTINIT