Add two startup options:
- --client_debug that turns out debug logging from the client
- --connect_timeout_secs that controls the timeout of the initial Ping() RPC from the client to the server
--
MOS_MIGRATED_REVID=138491791
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index da7b100..39215a5 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -231,7 +231,7 @@
// documentation is in command_server.proto .
class GrpcBlazeServer : public BlazeServer {
public:
- GrpcBlazeServer();
+ GrpcBlazeServer(int connect_timeout_secs);
virtual ~GrpcBlazeServer();
virtual bool Connect();
@@ -253,6 +253,7 @@
// a memory fence.
std::mutex cancel_thread_mutex_;
+ int connect_timeout_secs_;
int recv_socket_; // Socket the cancel thread reads actions from
int send_socket_; // Socket the main thread writes actions to
@@ -265,6 +266,19 @@
////////////////////////////////////////////////////////////////////////
// Logic
+void debug_log(const char* format, ...) {
+ if (!globals->options->client_debug) {
+ return;
+ }
+
+ fprintf(stderr, "CLIENT: ");
+ va_list arglist;
+ va_start(arglist, format);
+ vfprintf(stderr, format, arglist);
+ va_end(arglist);
+ fprintf(stderr, "%s", "\n");
+ fflush(stderr);
+}
#if !defined(__CYGWIN__)
// Returns the canonical form of the base dir given a root and a hashable
@@ -456,6 +470,9 @@
// JVM arguments are complete. Now pass in Blaze startup options.
// Note that we always use the --flag=ARG form (instead of the --flag ARG one)
// so that BlazeRuntime#splitStartupOptions has an easy job.
+
+ // TODO(lberki): Test that whatever the list constructed after this line is
+ // actually a list of parseable startup options.
if (!globals->options->batch) {
result.push_back("--max_idle_secs=" +
ToString(globals->options->max_idle_secs));
@@ -470,6 +487,10 @@
"--command_port=" + ToString(globals->options->command_port));
}
+ result.push_back(
+ "--connect_timeout_secs=" +
+ ToString(globals->options->connect_timeout_secs));
+
result.push_back("--install_base=" +
blaze::ConvertPath(globals->options->install_base));
result.push_back("--install_md5=" + globals->install_md5);
@@ -505,6 +526,18 @@
result.push_back("--nofatal_event_bus_exceptions");
}
+ // We use this syntax so that the logic in ServerNeedsToBeKilled() that
+ // decides whether the server needs killing is simpler. This is parsed by the
+ // Java code where --noclient_debug and --client_debug=false are equivalent.
+ // Note that --client_debug false (separated by space) won't work either,
+ // because the logic in ServerNeedsToBeKilled() assumes that every argument
+ // is in the --arg=value form.
+ if (globals->options->client_debug) {
+ result.push_back("--client_debug=true");
+ } else {
+ result.push_back("--client_debug=false");
+ }
+
// This is only for Blaze reporting purposes; the real interpretation of the
// jvm flags occurs when we set up the java command line.
if (globals->options->host_jvm_debug) {
@@ -782,15 +815,18 @@
for (int ii = 0; ii < 600; ++ii) {
// 60s; enough time to connect with debugger
if (server->Connect()) {
- if (ii) {
+ if (ii && !globals->options->client_debug) {
fputc('\n', stderr);
fflush(stderr);
}
delete server_startup;
return;
}
- fputc('.', stderr);
- fflush(stderr);
+ if (!globals->options->client_debug) {
+ fputc('.', stderr);
+ fflush(stderr);
+ }
+
struct timespec ts;
ts.tv_sec = 0;
ts.tv_nsec = 100 * 1000 * 1000;
@@ -1037,6 +1073,14 @@
}
}
+const char *volatile_startup_options[] = {
+ "--option_sources=",
+ "--max_idle_secs=",
+ "--connect_timeout_secs=",
+ "--client_debug=",
+ NULL,
+};
+
// Returns true if the server needs to be restarted to accommodate changes
// between the two argument lists.
static bool ServerNeedsToBeKilled(const vector<string>& args1,
@@ -1051,19 +1095,19 @@
}
for (int i = 0; i < args1.size(); i++) {
- string option_sources = "--option_sources=";
- if (args1[i].substr(0, option_sources.size()) == option_sources &&
- args2[i].substr(0, option_sources.size()) == option_sources) {
- continue;
+ bool option_volatile = false;
+ for (const char** candidate = volatile_startup_options;
+ *candidate != NULL;
+ candidate++) {
+ string candidate_string(*candidate);
+ if (args1[i].substr(0, candidate_string.size()) == candidate_string &&
+ args2[i].substr(0, candidate_string.size()) == candidate_string) {
+ option_volatile = true;
+ break;
+ }
}
- string max_idle_secs = "--max_idle_secs=";
- if (args1[i].substr(0, max_idle_secs.size()) == max_idle_secs &&
- args2[i].substr(0, max_idle_secs.size()) == max_idle_secs) {
- continue;
- }
-
- if (args1[i] != args2[i]) {
+ if (!option_volatile && args1[i] != args2[i]) {
return true;
}
}
@@ -1490,13 +1534,16 @@
CheckBinaryPath(argv[0]);
ParseOptions(argc, argv);
+ debug_log("Debug logging active");
+
CheckEnvironment();
CreateSecureOutputRoot();
const string self_path = GetSelfPath();
ComputeBaseDirectories(self_path);
- blaze_server = static_cast<BlazeServer *>(new GrpcBlazeServer());
+ blaze_server = static_cast<BlazeServer *>(new GrpcBlazeServer(
+ globals->options->connect_timeout_secs));
globals->command_wait_time = blaze_server->AcquireLock();
@@ -1522,9 +1569,12 @@
static void null_grpc_log_function(gpr_log_func_args *args) {
}
-GrpcBlazeServer::GrpcBlazeServer() {
- gpr_set_log_function(null_grpc_log_function);
+GrpcBlazeServer::GrpcBlazeServer(int connect_timeout_secs) {
connected_ = false;
+ connect_timeout_secs_ = connect_timeout_secs;
+
+ gpr_set_log_function(null_grpc_log_function);
+
int fd[2];
if (pipe(fd) < 0) {
pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
@@ -1584,15 +1634,20 @@
grpc::ClientContext context;
context.set_deadline(
- std::chrono::system_clock::now() + std::chrono::seconds(10));
+ std::chrono::system_clock::now() +
+ std::chrono::seconds(connect_timeout_secs_));
command_server::PingRequest request;
command_server::PingResponse response;
request.set_cookie(request_cookie_);
+ debug_log("Trying to connect to server (timeout: %d secs)...",
+ connect_timeout_secs_);
grpc::Status status = client->Ping(&context, request, &response);
if (!status.ok() || response.cookie() != response_cookie_) {
+ debug_log("Connection to server failed: %s",
+ status.error_message().c_str());
return false;
}
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 20c2eb9..9b048d9 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -51,7 +51,9 @@
allow_configurable_attributes(false),
fatal_event_bus_exceptions(false),
command_port(0),
- invocation_policy(NULL) {
+ connect_timeout_secs(10),
+ invocation_policy(NULL),
+ client_debug(false) {
bool testing = getenv("TEST_TMPDIR") != NULL;
if (testing) {
output_root = MakeAbsolute(getenv("TEST_TMPDIR"));
@@ -69,12 +71,12 @@
"host_jvm_debug", "master_blazerc", "master_bazelrc", "batch",
"batch_cpu_scheduling", "allow_configurable_attributes",
"fatal_event_bus_exceptions", "experimental_oom_more_eagerly",
- "write_command_log", "watchfs"};
+ "write_command_log", "watchfs", "client_debug"};
unary_options = {"output_base", "install_base",
"output_user_root", "host_jvm_profile", "host_javabase",
"host_jvm_args", "bazelrc", "blazerc", "io_nice_level",
"max_idle_secs", "experimental_oom_more_eagerly_threshold",
- "command_port", "invocation_policy"};
+ "command_port", "invocation_policy", "connect_timeout_secs"};
}
StartupOptions::~StartupOptions() {}
@@ -250,6 +252,23 @@
} else if (GetNullaryOption(arg, "--nowatchfs")) {
watchfs = false;
option_sources["watchfs"] = rcfile;
+ } else if (GetNullaryOption(arg, "--client_debug")) {
+ client_debug = true;
+ option_sources["client_debug"] = rcfile;
+ } else if (GetNullaryOption(arg, "--noclient_debug")) {
+ client_debug = false;
+ option_sources["client_debug"] = rcfile;
+ } else if ((value = GetUnaryOption(
+ arg, next_arg, "--connect_timeout_secs")) != NULL) {
+ if (!blaze_util::safe_strto32(value, &connect_timeout_secs) ||
+ connect_timeout_secs < 1 || connect_timeout_secs > 120) {
+ blaze_util::StringPrintf(error,
+ "Invalid argument to --connect_timeout_secs: '%s'.\n"
+ "Must be an integer between 1 and 120.\n",
+ value);
+ return blaze_exit_code::BAD_ARGV;
+ }
+ option_sources["connect_timeout_secs"] = rcfile;
} else if ((value = GetUnaryOption(
arg, next_arg, "--command_port")) != NULL) {
if (!blaze_util::safe_strto32(value, &command_port) ||
@@ -260,7 +279,7 @@
value);
return blaze_exit_code::BAD_ARGV;
}
- option_sources["webstatusserver"] = rcfile;
+ option_sources["command_port"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--invocation_policy"))
!= NULL) {
if (invocation_policy == NULL) {
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index db9bfb2..d0cd804 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -200,9 +200,15 @@
// gRPC command server.
int command_port;
+ // Connection timeout for each gRPC connection attempt.
+ int connect_timeout_secs;
+
// Invocation policy proto. May be NULL.
const char *invocation_policy;
+ // Whether to output addition debugging information in the client.
+ bool client_debug;
+
protected:
// Constructor for subclasses only so that site-specific extensions of this
// class can override the product name. The product_name must be the
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index 1849c36..64a9da7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -274,4 +274,16 @@
category = "undocumented",
help = "Whether or not to write the command.log file")
public boolean writeCommandLog;
+
+ @Option(name = "client_debug",
+ defaultValue = "false", // NOTE: purely decorative!
+ category = "server startup",
+ help = "If true, log debug information from the client to stderr")
+ public boolean clientDebug;
+
+ @Option(name = "connect_timeout_secs",
+ defaultValue = "10",
+ category = "server startup",
+ help = "The amount of time the client waits for each attempt to connect to the server")
+ public int connectTimeoutSecs;
}
