You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/10/24 23:19:09 UTC
[5/8] mesos git commit: Libprocess Reinit: Moved MetricsProcess
instantiation into process.cpp.
Libprocess Reinit: Moved MetricsProcess instantiation into process.cpp.
The metrics singleton must be unified with `process::initialize` so
that it also falls under the scope of reinitialization. The singleton
must also not be guarded by `Once`.
Review: https://reviews.apache.org/r/40410/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e705d5fd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e705d5fd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e705d5fd
Branch: refs/heads/master
Commit: e705d5fd2e86d23dcfafc9cefcad85a184da5565
Parents: 0a64d7a
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Mon Oct 24 15:06:35 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Mon Oct 24 16:18:50 2016 -0700
----------------------------------------------------------------------
.../include/process/metrics/metrics.hpp | 29 ++---
3rdparty/libprocess/src/metrics/metrics.cpp | 107 +++++++------------
3rdparty/libprocess/src/process.cpp | 14 ++-
3rdparty/libprocess/src/tests/system_tests.cpp | 4 +-
4 files changed, 69 insertions(+), 85 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e705d5fd/3rdparty/libprocess/include/process/metrics/metrics.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/metrics.hpp b/3rdparty/libprocess/include/process/metrics/metrics.hpp
index 54487ab..d9c64b7 100644
--- a/3rdparty/libprocess/include/process/metrics/metrics.hpp
+++ b/3rdparty/libprocess/include/process/metrics/metrics.hpp
@@ -29,16 +29,12 @@
namespace process {
namespace metrics {
-
-// Initializes the metrics library.
-void initialize(const Option<std::string>& authenticationRealm = None());
-
namespace internal {
class MetricsProcess : public Process<MetricsProcess>
{
public:
- static MetricsProcess* instance();
+ static MetricsProcess* create(const Option<std::string>& authenticationRealm);
Future<Nothing> add(Owned<Metric> metric);
@@ -83,24 +79,27 @@ private:
// Used to rate limit the snapshot endpoint.
Option<Owned<RateLimiter>> limiter;
- // Needed for access to the private constructor.
- friend void process::metrics::initialize(
- const Option<std::string>& authenticationRealm);
-
// The authentication realm that metrics HTTP endpoints are installed into.
const Option<std::string> authenticationRealm;
};
+
+// Global metrics process. Defined in process.cpp.
+extern PID<MetricsProcess> metrics;
+
} // namespace internal {
template <typename T>
Future<Nothing> add(const T& metric)
{
+ // The metrics process is instantiated in `process::initialize`.
+ process::initialize();
+
// There is an explicit copy in this call to ensure we end up owning
// the last copy of a Metric when we remove it.
return dispatch(
- internal::MetricsProcess::instance(),
+ internal::metrics,
&internal::MetricsProcess::add,
Owned<Metric>(new T(metric)));
}
@@ -108,8 +107,11 @@ Future<Nothing> add(const T& metric)
inline Future<Nothing> remove(const Metric& metric)
{
+ // The metrics process is instantiated in `process::initialize`.
+ process::initialize();
+
return dispatch(
- internal::MetricsProcess::instance(),
+ internal::metrics,
&internal::MetricsProcess::remove,
metric.name());
}
@@ -118,8 +120,11 @@ inline Future<Nothing> remove(const Metric& metric)
inline Future<hashmap<std::string, double>> snapshot(
const Option<Duration>& timeout)
{
+ // The metrics process is instantiated in `process::initialize`.
+ process::initialize();
+
return dispatch(
- internal::MetricsProcess::instance(),
+ internal::metrics,
&internal::MetricsProcess::snapshot,
timeout);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e705d5fd/3rdparty/libprocess/src/metrics/metrics.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/metrics/metrics.cpp b/3rdparty/libprocess/src/metrics/metrics.cpp
index 4ab3ac7..29c8c1d 100644
--- a/3rdparty/libprocess/src/metrics/metrics.cpp
+++ b/3rdparty/libprocess/src/metrics/metrics.cpp
@@ -19,7 +19,6 @@
#include <process/collect.hpp>
#include <process/dispatch.hpp>
#include <process/help.hpp>
-#include <process/once.hpp>
#include <process/owned.hpp>
#include <process/process.hpp>
@@ -39,85 +38,57 @@ using std::vector;
namespace process {
namespace metrics {
+namespace internal {
-
-static internal::MetricsProcess* metrics_process = nullptr;
-
-
-void initialize(const Option<string>& authenticationRealm)
+MetricsProcess* MetricsProcess::create(
+ const Option<string>& authenticationRealm)
{
- // To prevent a deadlock, we must ensure libprocess is
- // initialized. Otherwise, libprocess will be implicitly
- // initialized inside the 'once' block below, which in
- // turns initializes metrics, and we arrive back here
- // and deadlock by calling 'once()' without allowing
- // 'done()' to ever be called.
- process::initialize();
-
- static Once* initialized = new Once();
- if (!initialized->once()) {
- Option<string> limit =
- os::getenv("LIBPROCESS_METRICS_SNAPSHOT_ENDPOINT_RATE_LIMIT");
-
- Option<Owned<RateLimiter>> limiter;
-
- // By default, we apply a rate limit of 2 requests
- // per second to the metrics snapshot endpoint in
- // order to maintain backwards compatibility (before
- // this was made configurable, we hard-coded a limit
- // of 2 requests per second).
- if (limit.isNone()) {
- limiter = Owned<RateLimiter>(new RateLimiter(2, Seconds(1)));
- } else if (limit->empty()) {
- limiter = None();
- } else {
- // TODO(vinod): Move this parsing logic to flags
- // once we have a 'Rate' abstraction in stout.
- Option<Error> reason;
- vector<string> tokens = strings::tokenize(limit.get(), "/");
-
- if (tokens.size() == 2) {
- Try<int> requests = numify<int>(tokens[0]);
- Try<Duration> interval = Duration::parse(tokens[1]);
-
- if (requests.isError()) {
- reason = Error(
- "Failed to parse the number of requests: " + requests.error());
- } else if (interval.isError()) {
- reason = Error(
- "Failed to parse the interval: " + interval.error());
- } else {
- limiter = Owned<RateLimiter>(
- new RateLimiter(requests.get(), interval.get()));
- }
+ Option<string> limit =
+ os::getenv("LIBPROCESS_METRICS_SNAPSHOT_ENDPOINT_RATE_LIMIT");
+
+ Option<Owned<RateLimiter>> limiter;
+
+ // By default, we apply a rate limit of 2 requests
+ // per second to the metrics snapshot endpoint in
+ // order to maintain backwards compatibility (before
+ // this was made configurable, we hard-coded a limit
+ // of 2 requests per second).
+ if (limit.isNone()) {
+ limiter = Owned<RateLimiter>(new RateLimiter(2, Seconds(1)));
+ } else if (limit->empty()) {
+ limiter = None();
+ } else {
+ // TODO(vinod): Move this parsing logic to flags
+ // once we have a 'Rate' abstraction in stout.
+ Option<Error> reason;
+ vector<string> tokens = strings::tokenize(limit.get(), "/");
+
+ if (tokens.size() == 2) {
+ Try<int> requests = numify<int>(tokens[0]);
+ Try<Duration> interval = Duration::parse(tokens[1]);
+
+ if (requests.isError()) {
+ reason = Error(
+ "Failed to parse the number of requests: " + requests.error());
+ } else if (interval.isError()) {
+ reason = Error(
+ "Failed to parse the interval: " + interval.error());
+ } else {
+ limiter = Owned<RateLimiter>(
+ new RateLimiter(requests.get(), interval.get()));
}
+ }
- if (limiter.isNone()) {
+ if (limiter.isNone()) {
EXIT(EXIT_FAILURE)
<< "Failed to parse LIBPROCESS_METRICS_SNAPSHOT_ENDPOINT_RATE_LIMIT "
<< "'" << limit.get() << "'"
<< " (format is <number of requests>/<interval duration>)"
<< (reason.isSome() ? ": " + reason.get().message : "");
- }
}
-
- metrics_process =
- new internal::MetricsProcess(limiter, authenticationRealm);
- spawn(metrics_process);
-
- initialized->done();
}
-}
-
-
-namespace internal {
-
-
-MetricsProcess* MetricsProcess::instance()
-{
- metrics::initialize();
- return metrics_process;
+ return new MetricsProcess(limiter, authenticationRealm);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e705d5fd/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 2be8e84..8af056e 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -545,6 +545,14 @@ THREAD_LOCAL ProcessBase* __process__ = nullptr;
// Per thread executor pointer.
THREAD_LOCAL Executor* _executor_ = nullptr;
+namespace metrics {
+namespace internal {
+
+PID<metrics::internal::MetricsProcess> metrics;
+
+} // namespace internal {
+} // namespace metrics {
+
namespace http {
@@ -1079,8 +1087,10 @@ bool initialize(
// Create global help process.
help = spawn(new Help(delegate), true);
- // Initialize the global metrics process.
- metrics::initialize(readonlyAuthenticationRealm);
+ // Create the global metrics process.
+ metrics::internal::metrics = spawn(
+ metrics::internal::MetricsProcess::create(readonlyAuthenticationRealm),
+ true);
// Create the global logging process.
_logging = spawn(new Logging(readwriteAuthenticationRealm), true);
http://git-wip-us.apache.org/repos/asf/mesos/blob/e705d5fd/3rdparty/libprocess/src/tests/system_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/system_tests.cpp b/3rdparty/libprocess/src/tests/system_tests.cpp
index 0f4d042..6beb973 100644
--- a/3rdparty/libprocess/src/tests/system_tests.cpp
+++ b/3rdparty/libprocess/src/tests/system_tests.cpp
@@ -29,8 +29,6 @@ namespace http = process::http;
using process::Future;
-using process::metrics::internal::MetricsProcess;
-
// MESOS-1433
// This test is disabled as the Gauges that are used for these metrics
// may return Failures. In this case we do not put the metric into the
@@ -41,7 +39,7 @@ using process::metrics::internal::MetricsProcess;
TEST(SystemTest, DISABLED_Metrics)
{
Future<http::Response> response =
- http::get(MetricsProcess::instance()->self(), "snapshot");
+ http::get(process::metrics::internal::metrics, "snapshot");
AWAIT_READY(response);