Skip to content

Commit

Permalink
Rethink INFO/NOTICE separation based on more comments
Browse files Browse the repository at this point in the history 
Promoted some of the worse situations' messages into WARNING, as per
the discussion on rethinkdb#3040.
  • Loading branch information
sturmer committed Oct 1, 2014
1 parent 2968688 commit 16494b4
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/clustering/administration/http/directory_app.cc
View file
Expand Up @@ -101,10 +101,10 @@ void directory_http_app_t::handle(const http_req_t &req, http_res_t *result, sig
*result = http_error_res("Machine not found", HTTP_NOT_FOUND);
}
} catch (const schema_mismatch_exc_t &e) {
logNTC("HTTP request threw a schema_mismatch_exc_t with what = %s", e.what());
logWRN("HTTP request threw a schema_mismatch_exc_t with what = %s", e.what());
*result = http_error_res(e.what(), HTTP_NOT_FOUND);
} catch (const permission_denied_exc_t &e) {
logNTC("HTTP request threw a permission_denied_exc_t with what = %s", e.what());
logWRN("HTTP request threw a permission_denied_exc_t with what = %s", e.what());
// TODO: should that be 405 Method Not Allowed?
*result = http_error_res(e.what(), HTTP_FORBIDDEN);
}
Expand Down
18 changes: 9 additions & 9 deletions src/clustering/administration/http/semilattice_app.cc
View file
Expand Up @@ -77,7 +77,7 @@ void semilattice_http_app_t<metadata_t>::handle(const http_req_t &req, http_res_
#endif
scoped_cJSON_t change(cJSON_Parse(req.body.c_str()));
if (!change.get()) { //A null value indicates that parsing failed
logNTC("Json body failed to parse. Here's the data that failed: %s",
logWRN("Json body failed to parse. Here's the data that failed: %s",
req.get_sanitized_body().c_str());
*result = http_res_t(HTTP_BAD_REQUEST);
return;
Expand All @@ -94,7 +94,7 @@ void semilattice_http_app_t<metadata_t>::handle(const http_req_t &req, http_res_
str_to_uuid(prefer_distribution_param.get());
prioritize_distr_for_ns.reset(ns_id);
} catch (...) {
logNTC("Invalid value for prefer_distribution_for argument: %s",
logWRN("Invalid value for prefer_distribution_for argument: %s",
prefer_distribution_param.get().c_str());
*result = http_res_t(HTTP_BAD_REQUEST);
return;
Expand Down Expand Up @@ -146,7 +146,7 @@ void semilattice_http_app_t<metadata_t>::handle(const http_req_t &req, http_res_
#endif
scoped_cJSON_t change(cJSON_Parse(req.body.c_str()));
if (!change.get()) { //A null value indicates that parsing failed
logNTC("Json body failed to parse. Here's the data that failed: %s",
logWRN("Json body failed to parse. Here's the data that failed: %s",
req.get_sanitized_body().c_str());
*result = http_res_t(HTTP_BAD_REQUEST);
return;
Expand Down Expand Up @@ -184,19 +184,19 @@ void semilattice_http_app_t<metadata_t>::handle(const http_req_t &req, http_res_
break;
}
} catch (const schema_mismatch_exc_t &e) {
logNTC("HTTP request throw a schema_mismatch_exc_t with what = %s", e.what());
logWRN("HTTP request throw a schema_mismatch_exc_t with what = %s", e.what());
*result = http_error_res(e.what());
} catch (const permission_denied_exc_t &e) {
logNTC("HTTP request throw a permission_denied_exc_t with what = %s", e.what());
logWRN("HTTP request throw a permission_denied_exc_t with what = %s", e.what());
*result = http_error_res(e.what());
} catch (const cannot_satisfy_goals_exc_t &e) {
logNTC("The server was given a set of goals for which it couldn't find a valid blueprint. %s", e.what());
logINF("The server was given a set of goals for which it couldn't find a valid blueprint. %s", e.what());
*result = http_error_res(e.what(), HTTP_INTERNAL_SERVER_ERROR);
} catch (const gone_exc_t &e) {
logNTC("HTTP request throw a gone_exc_t with what = %s", e.what());
logWRN("HTTP request throw a gone_exc_t with what = %s", e.what());
*result = http_error_res(e.what(), HTTP_GONE);
} catch (const multiple_choices_exc_t &e) {
logNTC("HTTP request failed to change semilattice data. %s", e.what());
logWRN("HTTP request failed to change semilattice data. %s", e.what());
*result = http_error_res(e.what(), HTTP_METHOD_NOT_ALLOWED);
}
}
Expand All @@ -209,7 +209,7 @@ bool semilattice_http_app_t<metadata_t>::verify_content_type(const http_req_t &r
// information, and e.g. send "application/json; charset=UTF-8" instead of "application/json"
if (!content_type || !boost::istarts_with(content_type.get(), expected_content_type)) {
std::string actual_content_type = (content_type ? content_type.get() : "<NONE>");
logNTC("Bad request, Content-Type should be %s, but is %s.", expected_content_type.c_str(), actual_content_type.c_str());
logWRN("Bad request, Content-Type should be %s, but is %s.", expected_content_type.c_str(), actual_content_type.c_str());
return false;
}

Expand Down
3 changes: 1 addition & 2 deletions src/clustering/administration/logger.cc
View file
Expand Up @@ -61,8 +61,7 @@ std::string format_log_message(const log_message_t &m, bool for_console) {
m.uptime.tv_nsec / THOUSAND,
format_log_level(m.level).c_str());
} else {
if (m.level != log_level_info &&
m.level != log_level_notice) {
if (m.level != log_level_info && m.level != log_level_notice) {
prepend = strprintf("%s: ", format_log_level(m.level).c_str());
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/clustering/administration/main/command_line.cc
View file
Expand Up @@ -710,7 +710,7 @@ void run_rethinkdb_create(const base_path_t &base_path,
get_auth_metadata_filename(base_path),
&auth_perfmon_collection,
auth_semilattice_metadata_t());
logINF("Our machine ID: %s\n", uuid_to_str(our_machine_id).c_str());
logNTC("Our machine ID: %s\n", uuid_to_str(our_machine_id).c_str());
logINF("Created directory '%s' and a metadata file inside it.\n", base_path.path().c_str());

This comment has been minimized.

Sign in to view

Copy link
@timmaxw

timmaxw Oct 1, 2014

This should probably be logNTC. The fact that a directory was created is relevant to an interactive user.
*result_out = true;
} catch (const metadata_persistence::file_in_use_exc_t &ex) {
Expand Down Expand Up @@ -782,8 +782,8 @@ void run_rethinkdb_serve(const base_path_t &base_path,
const cluster_semilattice_metadata_t *cluster_metadata,
directory_lock_t *data_directory_lock,
bool *const result_out) {
logINF("Running %s...\n", RETHINKDB_VERSION_STR);
logINF("Running on %s", uname_msr().c_str());
logNTC("Running %s...\n", RETHINKDB_VERSION_STR);
logNTC("Running on %s", uname_msr().c_str());
os_signal_cond_t sigint_cond;

logINF("Using cache size of %" PRIu64 " MB",
Expand Down
16 changes: 8 additions & 8 deletions src/clustering/administration/main/serve.cc
View file
Expand Up @@ -98,7 +98,7 @@ bool do_serve(io_backender_t *io_backender,
auth_metadata = auth_metadata_file->read_metadata();
}
#ifndef NDEBUG
logINF("Our machine ID is %s", uuid_to_str(machine_id).c_str());
logNTC("Our machine ID is %s", uuid_to_str(machine_id).c_str());
#endif

connectivity_cluster_t connectivity_cluster;
Expand Down Expand Up @@ -322,7 +322,7 @@ bool do_serve(io_backender_t *io_backender,
{
scoped_ptr_t<administrative_http_server_manager_t> admin_server_ptr;
if (serve_info.ports.http_admin_is_disabled) {
logINF("Administrative HTTP connections are disabled.\n");
logNTC("Administrative HTTP connections are disabled.\n");
} else {
// TODO: Pardon me what, but is this how we fail here?
guarantee(serve_info.ports.http_port < 65536);
Expand All @@ -345,7 +345,7 @@ bool do_serve(io_backender_t *io_backender,
}

const std::string addresses_string = serve_info.ports.get_addresses_string();
logINF("Listening on addresses: %s\n", addresses_string.c_str());
logNTC("Listening on addresses: %s\n", addresses_string.c_str());

if (!serve_info.ports.is_bind_all()) {
logINF("To fully expose RethinkDB on the network, bind to all addresses");
Expand All @@ -369,7 +369,7 @@ bool do_serve(io_backender_t *io_backender,
} else {
machine_name = machine_it->second.get_ref().name.get().str();
}
logINF("Server ready, \"%s\" %s\n",
logNTC("Server ready, \"%s\" %s\n",
machine_name.c_str(),
uuid_to_str(machine_id).c_str());

Expand Down Expand Up @@ -397,13 +397,13 @@ bool do_serve(io_backender_t *io_backender,
auth_metadata_persister->stop_and_flush(&non_interruptor);
}

logINF("Shutting down client connections...\n");
logNTC("Shutting down client connections...\n");
}
logINF("All client connections closed.\n");
logNTC("All client connections closed.\n");

logINF("Shutting down storage engine... (This may take a while if you had a lot of unflushed data in the writeback cache.)\n");
logNTC("Shutting down storage engine... (This may take a while if you had a lot of unflushed data in the writeback cache.)\n");
}
logINF("Storage engine shut down.\n");
logNTC("Storage engine shut down.\n");

} catch (const address_in_use_exc_t &ex) {
logERR("%s.\n", ex.what());
Expand Down

0 comments on commit 16494b4

Please sign in to comment.