thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-09-20 08:40:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix comments, naming, log about kill inability and better test

Browse Source
This commit is contained in:
alesapin 2018-11-23 11:08:35 +03:00
parent 2b2195d4de
commit d31d30c8e1
3 changed files with 39 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
dbms/src/Common/ShellCommand.cpp
View File

@ -4,6 +4,7 @@
#include <dlfcn.h>
#include <Common/Exception.h>
#include <Common/ShellCommand.h>
#include <common/logger_useful.h>
#include <IO/WriteBufferFromVector.h>
#include <IO/WriteHelpers.h>
#include <port/unistd.h>
@ -76,15 +77,27 @@ namespace
namespace DB
{
ShellCommand::ShellCommand(pid_t pid, int in_fd, int out_fd, int err_fd, bool terminate_in_destructor_)
: pid(pid)
, terminate_in_destructor(terminate_in_destructor_)
, log(&Poco::Logger::get("ShellCommand"))
, in(in_fd)
, out(out_fd)
, err(err_fd) {}
ShellCommand::~ShellCommand()
{
if (die_in_destructor)
kill(pid, SIGTERM);
if (terminate_in_destructor)
{
int retcode = kill(pid, SIGTERM);
if (retcode != 0)
LOG_WARNING(log, "Cannot kill pid " << pid << " errno '" << errnoToString(retcode) << "'");
}
else if (!wait_called)
tryWait();
}
std::unique_ptr<ShellCommand> ShellCommand::executeImpl(const char * filename, char * const argv[], bool pipe_stdin_only, bool die_in_destructor)
std::unique_ptr<ShellCommand> ShellCommand::executeImpl(const char * filename, char * const argv[], bool pipe_stdin_only, bool terminate_in_destructor)
{
/** Here it is written that with a normal call `vfork`, there is a chance of deadlock in multithreaded programs,
* because of the resolving of characters in the shared library
@ -131,7 +144,7 @@ std::unique_ptr<ShellCommand> ShellCommand::executeImpl(const char * filename, c
_exit(int(ReturnCodes::CANNOT_EXEC));
}
std::unique_ptr<ShellCommand> res(new ShellCommand(pid, pipe_stdin.write_fd, pipe_stdout.read_fd, pipe_stderr.read_fd, die_in_destructor));
std::unique_ptr<ShellCommand> res(new ShellCommand(pid, pipe_stdin.write_fd, pipe_stdout.read_fd, pipe_stderr.read_fd, terminate_in_destructor));
/// Now the ownership of the file descriptors is passed to the result.
pipe_stdin.write_fd = -1;
@ -142,7 +155,7 @@ std::unique_ptr<ShellCommand> ShellCommand::executeImpl(const char * filename, c
}
std::unique_ptr<ShellCommand> ShellCommand::execute(const std::string & command, bool pipe_stdin_only, bool die_in_destructor)
std::unique_ptr<ShellCommand> ShellCommand::execute(const std::string & command, bool pipe_stdin_only, bool terminate_in_destructor)
{
/// Arguments in non-constant chunks of memory (as required for `execv`).
/// Moreover, their copying must be done before calling `vfork`, so after `vfork` do a minimum of things.
@ -152,11 +165,11 @@ std::unique_ptr<ShellCommand> ShellCommand::execute(const std::string & command,
char * const argv[] = { argv0.data(), argv1.data(), argv2.data(), nullptr };
return executeImpl("/bin/sh", argv, pipe_stdin_only, die_in_destructor);
return executeImpl("/bin/sh", argv, pipe_stdin_only, terminate_in_destructor);
}
std::unique_ptr<ShellCommand> ShellCommand::executeDirect(const std::string & path, const std::vector<std::string> & arguments, bool die_in_destructor)
std::unique_ptr<ShellCommand> ShellCommand::executeDirect(const std::string & path, const std::vector<std::string> & arguments, bool terminate_in_destructor)
{
size_t argv_sum_size = path.size() + 1;
for (const auto & arg : arguments)
@ -177,7 +190,7 @@ std::unique_ptr<ShellCommand> ShellCommand::executeDirect(const std::string & pa
argv[arguments.size() + 1] = nullptr;
return executeImpl(path.data(), argv.data(), false, die_in_destructor);
return executeImpl(path.data(), argv.data(), false, terminate_in_destructor);
}

17
dbms/src/Common/ShellCommand.h
View File

@ -28,12 +28,13 @@ class ShellCommand
private:
pid_t pid;
bool wait_called = false;
bool die_in_destructor;
bool terminate_in_destructor;
ShellCommand(pid_t pid, int in_fd, int out_fd, int err_fd, bool die_in_destructor_)
: pid(pid), die_in_destructor(die_in_destructor_), in(in_fd), out(out_fd), err(err_fd) {}
Poco::Logger * log;
static std::unique_ptr<ShellCommand> executeImpl(const char * filename, char * const argv[], bool pipe_stdin_only, bool die_in_destructor);
ShellCommand(pid_t pid, int in_fd, int out_fd, int err_fd, bool terminate_in_destructor_);
static std::unique_ptr<ShellCommand> executeImpl(const char * filename, char * const argv[], bool pipe_stdin_only, bool terminate_in_destructor);
public:
WriteBufferFromFile in; /// If the command reads from stdin, do not forget to call in.close() after writing all the data there.
@ -42,11 +43,13 @@ public:
~ShellCommand();
/// Run the command using /bin/sh -c
static std::unique_ptr<ShellCommand> execute(const std::string & command, bool pipe_stdin_only = false, bool die_in_destructor = false);
/// Run the command using /bin/sh -c.
/// If terminate_in_destructor is true, send terminate signal in destructor and don't wait process.
static std::unique_ptr<ShellCommand> execute(const std::string & command, bool pipe_stdin_only = false, bool terminate_in_destructor = false);
/// Run the executable with the specified arguments. `arguments` - without argv[0].
static std::unique_ptr<ShellCommand> executeDirect(const std::string & path, const std::vector<std::string> & arguments, bool die_in_destructor = false);
/// If terminate_in_destructor is true, send terminate signal in destructor and don't wait process.
static std::unique_ptr<ShellCommand> executeDirect(const std::string & path, const std::vector<std::string> & arguments, bool terminate_in_destructor = false);
/// Wait for the process to end, throw an exception if the code is not 0 or if the process was not completed by itself.
void wait();

11
dbms/tests/integration/test_odbc_interaction/test.py
View File

@ -194,10 +194,15 @@ def test_bridge_dies_with_parent(started_cluster):
assert clickhouse_pid is not None
assert bridge_pid is not None
node1.exec_in_container(["bash", "-c", "kill {}".format(clickhouse_pid)], privileged=True, user='root')
node1.exec_in_container(["bash", "-c", "kill {}".format(clickhouse_pid)], privileged=True, user='root')
while clickhouse_pid is not None:
try:
node1.exec_in_container(["bash", "-c", "kill {}".format(clickhouse_pid)], privileged=True, user='root')
except:
pass
clickhouse_pid = get_pid("clickhouse server")
time.sleep(1)
clickhouse_pid = get_pid("clickhouse server")
time.sleep(1) # just for sure, that odbc-bridge caught signal
bridge_pid = get_pid("odbc-bridge")
assert clickhouse_pid is None