You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2014/04/18 01:24:18 UTC
[1/2] git commit: Changed registrar tests to use LogStorage.
Repository: mesos
Updated Branches:
refs/heads/master f4ae816b6 -> 033d788fd
Changed registrar tests to use LogStorage.
Review: https://reviews.apache.org/r/20432
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/21f823df
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/21f823df
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/21f823df
Branch: refs/heads/master
Commit: 21f823df0de72c573ade7dc2624b2ab30d94f9a9
Parents: f4ae816
Author: Jiang Yan Xu <ya...@jxu.me>
Authored: Tue Apr 15 16:01:33 2014 -0700
Committer: Jiang Yan Xu <ya...@jxu.me>
Committed: Thu Apr 17 16:20:46 2014 -0700
----------------------------------------------------------------------
src/tests/registrar_tests.cpp | 129 +++++++++++++++++++++----------------
1 file changed, 74 insertions(+), 55 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/21f823df/src/tests/registrar_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/registrar_tests.cpp b/src/tests/registrar_tests.cpp
index c0ef0ca..7afa223 100644
--- a/src/tests/registrar_tests.cpp
+++ b/src/tests/registrar_tests.cpp
@@ -18,6 +18,7 @@
#include <algorithm>
#include <map>
+#include <set>
#include <string>
#include <vector>
@@ -32,64 +33,120 @@
#include "common/protobuf_utils.hpp"
#include "common/type_utils.hpp"
+#include "log/log.hpp"
+#include "log/replica.hpp"
+
+#include "log/tool/initialize.hpp"
+
#include "master/flags.hpp"
#include "master/master.hpp"
#include "master/registrar.hpp"
-#include "state/in_memory.hpp"
-#include "state/leveldb.hpp"
+#include "state/log.hpp"
#include "state/protobuf.hpp"
#include "state/storage.hpp"
+#include "tests/utils.hpp"
+
using namespace mesos;
using namespace mesos::internal;
using namespace process;
+using state::protobuf::State;
+
using std::map;
+using std::set;
using std::string;
using std::vector;
using testing::_;
using testing::Eq;
+using mesos::internal::tests::TemporaryDirectoryTest;
+
+using ::testing::WithParamInterface;
+
namespace mesos {
namespace internal {
namespace master {
-class RegistrarTest : public ::testing::TestWithParam<bool>
+// TODO(xujyan): This class copies code from LogStateTest. It would
+// be nice to find a common location for log related base tests when
+// there are more uses of it.
+class RegistrarTestBase : public TemporaryDirectoryTest
{
public:
- RegistrarTest()
- : storage(NULL),
- state(NULL) {}
+ RegistrarTestBase()
+ : log(NULL),
+ storage(NULL),
+ state(NULL),
+ replica2(NULL) {}
protected:
virtual void SetUp()
{
- // We use InMemoryStorage to test Registrar correctness and
- // LevelDBStorage to test performance.
- // TODO(xujyan): Use LogStorage to exercise what we're using in
- // production.
- storage = new state::InMemoryStorage();
- state = new state::protobuf::State(storage);
+ TemporaryDirectoryTest::SetUp();
+
+ // For initializing the replicas.
+ log::tool::Initialize initializer;
+
+ string path1 = os::getcwd() + "/.log1";
+ string path2 = os::getcwd() + "/.log2";
+
+ initializer.flags.path = path1;
+ initializer.execute();
+
+ initializer.flags.path = path2;
+ initializer.execute();
+
+ // Only create the replica for 'path2' (i.e., the second replica)
+ // as the first replica will be created when we create a Log.
+ replica2 = new log::Replica(path2);
+
+ set<UPID> pids;
+ pids.insert(replica2->pid());
+
+ log = new log::Log(2, path1, pids);
+ storage = new state::LogStorage(log);
+ state = new State(storage);
+
master.CopyFrom(protobuf::createMasterInfo(UPID("master@127.0.0.1:5050")));
- flags.registry_strict = GetParam();
}
virtual void TearDown()
{
delete state;
delete storage;
+ delete log;
+ delete replica2;
+
+ TemporaryDirectoryTest::TearDown();
}
+ log::Log* log;
state::Storage* storage;
- state::protobuf::State* state;
+ State* state;
+
+ log::Replica* replica2;
+
MasterInfo master;
Flags flags;
};
+class RegistrarTest : public RegistrarTestBase,
+ public WithParamInterface<bool>
+{
+protected:
+ virtual void SetUp()
+ {
+ RegistrarTestBase::SetUp();
+ flags.registry_strict = GetParam();
+ }
+};
+
+
// The Registrar tests are parameterized by "strictness".
INSTANTIATE_TEST_CASE_P(Strict, RegistrarTest, ::testing::Bool());
@@ -285,47 +342,9 @@ TEST_P(RegistrarTest, bootstrap)
}
-// We are not inheriting from RegistrarTest because this test fixture
-// derives from a different instantiation of the TestWithParam template.
-class Registrar_BENCHMARK_Test : public ::testing::TestWithParam<size_t>
-{
-public:
- Registrar_BENCHMARK_Test()
- : storage(NULL),
- state(NULL),
- path(os::getcwd() + "/.state") {}
-
-protected:
- virtual void SetUp()
- {
- os::rmdir(path);
-
- // We use InMemoryStorage to test Registrar correctness and
- // LevelDBStorage to test performance.
- storage = new state::LevelDBStorage(path);
- state = new state::protobuf::State(storage);
- master.CopyFrom(protobuf::createMasterInfo(UPID("master@127.0.0.1:5050")));
-
- // Strictness is not important in our benchmark tests so it's
- // just set to false here.
- flags.registry_strict = false;
- }
-
- virtual void TearDown()
- {
- delete state;
- delete storage;
- os::rmdir(path);
- }
-
- state::Storage* storage;
- state::protobuf::State* state;
- MasterInfo master;
- Flags flags;
-
-private:
- const std::string path;
-};
+class Registrar_BENCHMARK_Test : public RegistrarTestBase,
+ public WithParamInterface<size_t>
+{};
// The Registrar benchmark tests are parameterized by the number of slaves.
[2/2] git commit: Improved Registrar performance by using a hashset
on SlaveIDs.
Posted by ya...@apache.org.
Improved Registrar performance by using a hashset on SlaveIDs.
Review: https://reviews.apache.org/r/19762
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/033d788f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/033d788f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/033d788f
Branch: refs/heads/master
Commit: 033d788fda059345e715269f2fb9b2bea622be1d
Parents: 21f823d
Author: Jiang Yan Xu <ya...@jxu.me>
Authored: Thu Mar 27 11:27:40 2014 -0700
Committer: Jiang Yan Xu <ya...@jxu.me>
Committed: Thu Apr 17 16:20:47 2014 -0700
----------------------------------------------------------------------
src/master/master.hpp | 36 ++++++++++++++++++++++--------------
src/master/registrar.cpp | 14 ++++++++++++--
src/master/registrar.hpp | 41 ++++++++++++++++++++++++++++++++++-------
3 files changed, 68 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/033d788f/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index e4c7862..f567a43 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -785,21 +785,23 @@ public:
}
protected:
- virtual Try<bool> perform(Registry* registry, bool strict)
+ virtual Try<bool> perform(
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict)
{
// Check and see if this slave already exists.
- foreach (const Registry::Slave& slave, registry->slaves().slaves()) {
- if (slave.info().id() == info.id()) {
- if (strict) {
- return Error("Slave already admitted");
- } else {
- return false; // No mutation.
- }
+ if (slaveIDs->contains(info.id())) {
+ if (strict) {
+ return Error("Slave already admitted");
+ } else {
+ return false; // No mutation.
}
}
Registry::Slave* slave = registry->mutable_slaves()->add_slaves();
slave->mutable_info()->CopyFrom(info);
+ slaveIDs->insert(info.id());
return true; // Mutation.
}
@@ -818,12 +820,13 @@ public:
}
protected:
- virtual Try<bool> perform(Registry* registry, bool strict)
+ virtual Try<bool> perform(
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict)
{
- foreach (const Registry::Slave& slave, registry->slaves().slaves()) {
- if (slave.info().id() == info.id()) {
- return false; // No mutation.
- }
+ if (slaveIDs->contains(info.id())) {
+ return false; // No mutation.
}
if (strict) {
@@ -831,6 +834,7 @@ protected:
} else {
Registry::Slave* slave = registry->mutable_slaves()->add_slaves();
slave->mutable_info()->CopyFrom(info);
+ slaveIDs->insert(info.id());
return true; // Mutation.
}
}
@@ -850,12 +854,16 @@ public:
}
protected:
- virtual Try<bool> perform(Registry* registry, bool strict)
+ virtual Try<bool> perform(
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict)
{
for (int i = 0; i < registry->slaves().slaves().size(); i++) {
const Registry::Slave& slave = registry->slaves().slaves(i);
if (slave.info().id() == info.id()) {
registry->mutable_slaves()->mutable_slaves()->DeleteSubrange(i, 1);
+ slaveIDs->erase(info.id());
return true; // Mutation.
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/033d788f/src/master/registrar.cpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.cpp b/src/master/registrar.cpp
index 9c0537d..38040bd 100644
--- a/src/master/registrar.cpp
+++ b/src/master/registrar.cpp
@@ -105,7 +105,10 @@ private:
explicit Recover(const MasterInfo& _info) : info(_info) {}
protected:
- virtual Try<bool> perform(Registry* registry, bool strict)
+ virtual Try<bool> perform(
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict)
{
registry->mutable_master()->mutable_info()->CopyFrom(info);
return true; // Mutation.
@@ -312,11 +315,18 @@ void RegistrarProcess::update()
CHECK_SOME(variable);
+ // Create a snapshot of the current registry.
Registry registry = variable.get().get();
+ // Create the 'slaveIDs' accumulator.
+ hashset<SlaveID> slaveIDs;
+ foreach (const Registry::Slave& slave, registry.slaves().slaves()) {
+ slaveIDs.insert(slave.info().id());
+ }
+
foreach (Owned<Operation> operation, operations) {
// No need to process the result of the operation.
- (*operation)(®istry, flags.registry_strict);
+ (*operation)(®istry, &slaveIDs, flags.registry_strict);
}
// TODO(benh): Add a timeout so we don't wait forever.
http://git-wip-us.apache.org/repos/asf/mesos/blob/033d788f/src/master/registrar.hpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.hpp b/src/master/registrar.hpp
index 0d659c5..6bc78c4 100644
--- a/src/master/registrar.hpp
+++ b/src/master/registrar.hpp
@@ -21,6 +21,8 @@
#include <mesos/mesos.hpp>
+#include <stout/hashset.hpp>
+
#include <process/future.hpp>
#include <process/owned.hpp>
@@ -38,17 +40,25 @@ class RegistrarProcess;
// Defines an abstraction for operations that can be applied on the
// Registry.
+// TODO(xujyan): Make Operation generic so that we can apply them
+// against a generic "batch operation applier" abstraction, see TODO
+// below for more details.
class Operation : public process::Promise<bool>
{
public:
Operation() : success(false) {}
-
- // Attempts to invoke the operation on 't'.
- // Returns whether the operation mutates 't', or an error if the
- // operation cannot be applied successfully.
- Try<bool> operator () (Registry* registry, bool strict)
+ virtual ~Operation() {}
+
+ // Attempts to invoke the operation on 'registry' (and the
+ // accumulators, in this case 'slaveIDs').
+ // Returns whether the operation mutates 'registry', or an error if
+ // the operation cannot be applied successfully.
+ Try<bool> operator () (
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict)
{
- const Try<bool>& result = perform(registry, strict);
+ const Try<bool>& result = perform(registry, slaveIDs, strict);
success = !result.isError();
@@ -59,13 +69,30 @@ public:
bool set() { return process::Promise<bool>::set(success); }
protected:
- virtual Try<bool> perform(Registry* registry, bool strict) = 0;
+ virtual Try<bool> perform(
+ Registry* registry,
+ hashset<SlaveID>* slaveIDs,
+ bool strict) = 0;
private:
bool success;
};
+// TODO(xujyan): Add a generic abstraction for applying batches of
+// operations against State Variables. The Registrar and other
+// components could leverage this. This abstraction would be
+// templatized to take the type, along with any accumulators:
+// template <typename T,
+// typename X = None,
+// typename Y = None,
+// typename Z = None>
+// T: the data type that the batch operations can be applied on.
+// X, Y, Z: zero to 3 generic accumulators that facilitate the batch
+// of operations.
+// This allows us to reuse the idea of "doing batches of operations"
+// on other potential new state variables (i.e. repair state, offer
+// reservations, etc).
class Registrar
{
public: