You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2012/10/04 01:37:40 UTC
svn commit: r1393824 - in /incubator/mesos/trunk/src: detector/detector.cpp
tests/zookeeper_tests.cpp zookeeper/authentication.cpp
zookeeper/authentication.hpp zookeeper/group.cpp zookeeper/zookeeper.cpp
zookeeper/zookeeper.hpp
Author: benh
Date: Wed Oct 3 23:37:40 2012
New Revision: 1393824
URL: http://svn.apache.org/viewvc?rev=1393824&view=rev
Log:
Refactored recursive ZooKeeper path create code from Group into
ZooKeeper::create (which now takes a boolean 'recursive' flag that
determines whether or not it should try and create intermediate
znodes). Also refactored ZooKeeperMasterDetector to use this new
version of create (https://reviews.apache.org/r/7391).
Modified:
incubator/mesos/trunk/src/detector/detector.cpp
incubator/mesos/trunk/src/tests/zookeeper_tests.cpp
incubator/mesos/trunk/src/zookeeper/authentication.cpp
incubator/mesos/trunk/src/zookeeper/authentication.hpp
incubator/mesos/trunk/src/zookeeper/group.cpp
incubator/mesos/trunk/src/zookeeper/zookeeper.cpp
incubator/mesos/trunk/src/zookeeper/zookeeper.hpp
Modified: incubator/mesos/trunk/src/detector/detector.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/detector/detector.cpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/detector/detector.cpp (original)
+++ incubator/mesos/trunk/src/detector/detector.cpp Wed Oct 3 23:37:40 2012
@@ -301,57 +301,49 @@ void ZooKeeperMasterDetectorProcess::con
if (!reconnect) {
LOG(INFO) << "Master detector connected to ZooKeeper ...";
- int code;
if (url.authentication.isSome()) {
const std::string& scheme = url.authentication.get().scheme;
const std::string& credentials = url.authentication.get().credentials;
LOG(INFO) << "Authenticating to ZooKeeper using scheme '" << scheme << "'";
- code = zk->authenticate(scheme, credentials);
+ int code = zk->authenticate(scheme, credentials);
if (code != ZOK) {
LOG(FATAL) << "Failed to authenticate with ZooKeeper: "
<< zk->message(code);
}
}
- string result;
-
- static const string delimiter = "/";
-
// Assume the path (chroot) being used does not end with a "/".
CHECK(url.path.at(url.path.length() - 1) != '/');
- // Create znodes as necessary.
- size_t index = url.path.find(delimiter, 0);
-
- while (index < string::npos) {
- // Get out the prefix to create.
- index = url.path.find(delimiter, index + 1);
- string prefix = url.path.substr(0, index);
-
- LOG(INFO) << "Trying to create znode '" << prefix << "' in ZooKeeper";
+ // Create znode path (including intermediate znodes) as necessary.
+ LOG(INFO) << "Trying to create path '" << url.path << "' in ZooKeeper";
- // Create the node (even if it already exists).
- code = zk->create(prefix, "", acl, 0, &result);
+ int code = zk->create(url.path, "", acl, 0, NULL, true);
- if (code != ZOK && code != ZNODEEXISTS) {
- LOG(FATAL) << "Failed to create ZooKeeper znode: " << zk->message(code);
- }
- }
-
- // Wierdness in ZooKeeper timing, let's check that everything is created.
- code = zk->get(url.path, false, &result, NULL);
-
- if (code != ZOK) {
- LOG(FATAL) << "Unexpected ZooKeeper failure: " << zk->message(code);
+ // We fail all non-OK return codes except ZNODEEXISTS (since that
+ // means the path we were trying to create exists) and ZNOAUTH
+ // (since it's possible that the ACLs on 'dirname(url.path)' don't
+ // allow us to create a child znode but we are allowed to create
+ // children of 'url.path' itself, which will be determined below
+ // if we are contending). Note that it's also possible we got back
+ // a ZNONODE because we could not create one of the intermediate
+ // znodes (in which case we'll abort in the 'else' below since
+ // ZNONODE is non-retryable). TODO(benh): Need to check that we
+ // also can put a watch on the children of 'url.path'.
+ if (code != ZOK && code != ZNODEEXISTS && code != ZNOAUTH) {
+ LOG(FATAL) << "Failed to create '" << url.path
+ << "' in ZooKeeper: " << zk->message(code);
}
if (contend) {
// We contend with the pid given in constructor.
- code = zk->create(url.path + "/", pid, acl,
- ZOO_SEQUENCE | ZOO_EPHEMERAL, &result);
+ string result;
+ int code = zk->create(url.path + "/", pid, acl,
+ ZOO_SEQUENCE | ZOO_EPHEMERAL, &result);
if (code != ZOK) {
- LOG(FATAL) << "Unexpected ZooKeeper failure: %s" << zk->message(code);
+ LOG(FATAL) << "Unable to create ephemeral child of '" << url.path
+ << "' in ZooKeeper: %s" << zk->message(code);
}
// Save the sequence id but only grab the basename, e.g.,
Modified: incubator/mesos/trunk/src/tests/zookeeper_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/zookeeper_tests.cpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/zookeeper_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/zookeeper_tests.cpp Wed Oct 3 23:37:40 2012
@@ -27,6 +27,7 @@
#include <process/protobuf.hpp>
#include <stout/duration.hpp>
+#include <stout/strings.hpp>
#include <stout/try.hpp>
#include "detector/detector.hpp"
@@ -93,6 +94,54 @@ TEST_F(ZooKeeperTest, Auth)
}
+TEST_F(ZooKeeperTest, Create)
+{
+ BaseZooKeeperTest::TestWatcher watcher;
+
+ ZooKeeper authenticatedZk(zks->connectString(), NO_TIMEOUT, &watcher);
+ watcher.awaitSessionEvent(ZOO_CONNECTED_STATE);
+ authenticatedZk.authenticate("digest", "creator:creator");
+ EXPECT_EQ(ZOK, authenticatedZk.create("/foo/bar",
+ "",
+ zookeeper::EVERYONE_READ_CREATOR_ALL,
+ 0,
+ NULL,
+ true));
+ authenticatedZk.create("/foo/bar/baz",
+ "43",
+ zookeeper::EVERYONE_CREATE_AND_READ_CREATOR_ALL,
+ 0,
+ NULL);
+ assertGet(&authenticatedZk, "/foo/bar/baz", "43");
+
+ ZooKeeper nonOwnerZk(zks->connectString(), NO_TIMEOUT, &watcher);
+ watcher.awaitSessionEvent(ZOO_CONNECTED_STATE);
+ nonOwnerZk.authenticate("digest", "non-owner:non-owner");
+ EXPECT_EQ(ZNOAUTH, nonOwnerZk.create("/foo/bar/baz",
+ "",
+ zookeeper::EVERYONE_READ_CREATOR_ALL,
+ 0,
+ NULL,
+ true));
+ EXPECT_EQ(ZOK, nonOwnerZk.create("/foo/bar/baz/bam",
+ "44",
+ zookeeper::EVERYONE_READ_CREATOR_ALL,
+ 0,
+ NULL,
+ true));
+ assertGet(&nonOwnerZk, "/foo/bar/baz/bam", "44");
+
+ std::string result;
+ EXPECT_EQ(ZOK, nonOwnerZk.create("/foo/bar/baz/",
+ "",
+ zookeeper::EVERYONE_READ_CREATOR_ALL,
+ ZOO_SEQUENCE | ZOO_EPHEMERAL,
+ &result,
+ true));
+ EXPECT_TRUE(strings::startsWith(result, "/foo/bar/baz/0"));
+}
+
+
class MockMasterDetectorListenerProcess
: public ProtobufProcess<MockMasterDetectorListenerProcess>
{
Modified: incubator/mesos/trunk/src/zookeeper/authentication.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/authentication.cpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/authentication.cpp (original)
+++ incubator/mesos/trunk/src/zookeeper/authentication.cpp Wed Oct 3 23:37:40 2012
@@ -12,4 +12,16 @@ const ACL_vector EVERYONE_READ_CREATOR_A
2, _EVERYONE_READ_CREATOR_ALL_ACL
};
+
+ACL _EVERYONE_CREATE_AND_READ_CREATOR_ALL_ACL[] = {
+ { ZOO_PERM_CREATE, ZOO_ANYONE_ID_UNSAFE },
+ { ZOO_PERM_READ, ZOO_ANYONE_ID_UNSAFE },
+ { ZOO_PERM_ALL, ZOO_AUTH_IDS }
+};
+
+
+const ACL_vector EVERYONE_CREATE_AND_READ_CREATOR_ALL = {
+ 3, _EVERYONE_CREATE_AND_READ_CREATOR_ALL_ACL
+};
+
} // namespace zookeeper {
Modified: incubator/mesos/trunk/src/zookeeper/authentication.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/authentication.hpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/authentication.hpp (original)
+++ incubator/mesos/trunk/src/zookeeper/authentication.hpp Wed Oct 3 23:37:40 2012
@@ -24,6 +24,10 @@ struct Authentication
// nodes - others are welcome to read.
extern const ACL_vector EVERYONE_READ_CREATOR_ALL;
+// An ACL that allows others to create child nodes and read nodes, but
+// we're the the only authenticated user to mutate our nodes.
+extern const ACL_vector EVERYONE_CREATE_AND_READ_CREATOR_ALL;
+
} // namespace zookeeper {
#endif // __ZOOKEEPER_AUTHENTICATION_HPP__
Modified: incubator/mesos/trunk/src/zookeeper/group.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/group.cpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/group.cpp (original)
+++ incubator/mesos/trunk/src/zookeeper/group.cpp Wed Oct 3 23:37:40 2012
@@ -396,67 +396,40 @@ void GroupProcess::connected(bool reconn
}
}
- // Create directory path znodes as necessary.
+ // Create znode path (including intermediate znodes) as necessary.
CHECK(znode.size() == 0 || znode.at(znode.size() - 1) != '/');
- size_t index = znode.find("/", 0);
- while (index < string::npos) {
- // Get out the prefix to create.
- index = znode.find("/", index + 1);
- const string& prefix = znode.substr(0, index);
-
- LOG(INFO) << "Trying to create '" << prefix << "' in ZooKeeper";
-
- // Create the node (even if it already exists).
- int code = zk->create(prefix, "", acl, 0, NULL);
-
- if (code == ZINVALIDSTATE || (code != ZOK && zk->retryable(code))) {
- CHECK(zk->getState() != ZOO_AUTH_FAILED_STATE);
- return; // Try again later.
- } else if (code != ZOK && code != ZNODEEXISTS && code != ZNOAUTH) {
- // We fail all non-OK return codes except for ZNODEEXISTS and ZNOAUTH:
- // ZNODEEXISTS says the node in the znode path we are trying to create
- // already exists - this is what we wanted, so we continue.
- // ZNOAUTH says we can't write the node, but it doesn't tell us
- // whether the node already exists. We take the optimistic approach
- // and assume the node's parent doesn't allow us to write an already
- // existing node. As long as the last node in the znode path exists
- // (or we can create it) and we can write children to that last node,
- // we're good. This condition is tested below when we're done trying
- // to create the znode path.
- Try<string> message = strings::format(
- "Failed to create '%s' in ZooKeeper: %s",
- prefix, zk->message(code));
- error = message.isSome()
- ? message.get()
- : "Failed to create node in ZooKeeper";
- abort(); // Cancels everything pending.
- return;
- }
- }
+ LOG(INFO) << "Trying to create path '" << znode << "' in ZooKeeper";
+
+ int code = zk->create(znode, "", acl, 0, NULL, true);
- // Now check we have perms to write to the final znode - this is required
- // to run the Group.
- string result;
- int code = zk->create(znode + "/__write_test_", "", acl,
- ZOO_SEQUENCE | ZOO_EPHEMERAL, &result);
- if (code == ZINVALIDSTATE || (code != ZOK && zk->retryable(code))) {
+ // We fail all non-OK return codes except ZNONODEEXISTS (since
+ // that means the path we were trying to create exists) and
+ // ZNOAUTH (since it's possible that the ACLs on 'dirname(znode)'
+ // don't allow us to create a child znode but we are allowed to
+ // create children of 'znode' itself, which will be determined
+ // when we first do a Group::join). Note that it's also possible
+ // we got back a ZNONODE because we could not create one of the
+ // intermediate znodes (in which case we'll abort in the 'else'
+ // below since ZNONODE is non-retryable). TODO(benh): Need to
+ // check that we also can put a watch on the children of 'znode'.
+ if (code == ZINVALIDSTATE ||
+ (code != ZOK &&
+ code != ZNODEEXISTS &&
+ code != ZNOAUTH &&
+ zk->retryable(code))) {
CHECK(zk->getState() != ZOO_AUTH_FAILED_STATE);
return; // Try again later.
- } else if (code != ZOK) {
+ } else if (code != ZOK && code != ZNODEEXISTS && code != ZNOAUTH) {
Try<string> message = strings::format(
- "Unable to write to configured group path '%s' in ZooKeeper: %s",
+ "Failed to create '%s' in ZooKeeper: %s",
znode, zk->message(code));
error = message.isSome()
? message.get()
- : "Failed to create node in ZooKeeper";
+ : "Failed to create znode in ZooKeeper";
abort(); // Cancels everything pending.
return;
}
-
- // Make a best-effort only attempt to clean up our write test node -
- // it will die with our session if the attempts fails.
- zk->remove(result, -1);
}
state = CONNECTED;
@@ -841,6 +814,9 @@ void GroupProcess::abort()
fail(&pending.cancels, error.get());
fail(&pending.datas, error.get());
fail(&pending.watches, error.get());
+
+ // TODO(benh): Delete the ZooKeeper instance in order to terminate
+ // our session (cleaning up any ephemeral znodes as necessary)?
}
Modified: incubator/mesos/trunk/src/zookeeper/zookeeper.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/zookeeper.cpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/zookeeper.cpp (original)
+++ incubator/mesos/trunk/src/zookeeper/zookeeper.cpp Wed Oct 3 23:37:40 2012
@@ -28,6 +28,9 @@
#include <stout/duration.hpp>
#include <stout/fatal.hpp>
+#include <stout/foreach.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
#include "zookeeper/zookeeper.hpp"
@@ -515,9 +518,40 @@ int ZooKeeper::authenticate(const string
int ZooKeeper::create(const string& path, const string& data,
- const ACL_vector& acl, int flags, string* result)
+ const ACL_vector& acl, int flags, string* result,
+ bool recursive)
{
- return impl->create(path, data, acl, flags, result).get();
+ if (!recursive) {
+ return impl->create(path, data, acl, flags, result).get();
+ }
+
+ // Do "recursive" create, i.e., ensure intermediate znodes exist.
+ string prefix = "/";
+ int code = ZOK;
+ foreach (const string& token, strings::tokenize(path, "/")) {
+ prefix = path::join(prefix, token);
+
+ // Make sure we include 'flags' and 'data' for the final znode.
+ if (prefix == path || (prefix + "/") == path) {
+ code = impl->create(path, data, acl, flags, result).get();
+ } else {
+ code = impl->create(prefix, "", acl, 0, result).get();
+ }
+
+ // We fail all non-OK return codes except for:
+ // ZNODEEXISTS says the node in the znode path we are trying to
+ // create already exists - this is what we wanted, so we
+ // continue.
+ // ZNOAUTH says we can't write the node, but it doesn't tell us
+ // whether the node already exists. We take the optimistic
+ // approach and assume the node's parent doesn't allow us to
+ // write an already existing node (but it exists).
+ if (code != ZOK && code != ZNODEEXISTS && code != ZNOAUTH) {
+ return code;
+ }
+ }
+
+ return code;
}
Modified: incubator/mesos/trunk/src/zookeeper/zookeeper.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/zookeeper.hpp?rev=1393824&r1=1393823&r2=1393824&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/zookeeper.hpp (original)
+++ incubator/mesos/trunk/src/zookeeper/zookeeper.hpp Wed Oct 3 23:37:40 2012
@@ -166,6 +166,9 @@ public:
* the new node (this might be different than the supplied path
* because of the ZOO_SEQUENCE flag). The path string will always
* be null-terminated.
+ * \param recursive if true, attempts to create all intermediate
+ * znodes as required; note that 'flags' and 'data' will only be
+ * applied to the creation of 'basename(path)'.
* \return one of the following codes are returned:
* ZOK operation completed succesfully
* ZNONODE the parent node does not exist.
@@ -180,7 +183,8 @@ public:
const std::string &data,
const ACL_vector &acl,
int flags,
- std::string *result);
+ std::string *result,
+ bool recursive = false);
/**
* \brief delete a node in zookeeper synchronously.