From b960bf5ad4dff5121cdd7b6ed82111fa17d282df Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Mon, 27 Nov 2017 23:40:07 -0600 Subject: [PATCH 1/6] Grants the lease to the client when it creates the file. --- zookeeper/include/zk_nn_client.h | 2 +- zookeeper/source/zk_nn_client.cc | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/zookeeper/include/zk_nn_client.h b/zookeeper/include/zk_nn_client.h index 8c74d984..77a7210e 100644 --- a/zookeeper/include/zk_nn_client.h +++ b/zookeeper/include/zk_nn_client.h @@ -613,7 +613,7 @@ class ZkNnClient : public ZkClientCommon { * Crate a znode corresponding to a file of "filetype", with path "path", with * znode data contained in "znode_data" */ - bool create_file_znode(const std::string &path, FileZNode *znode_data); + bool create_file_znode(const std::string &path, FileZNode *znode_data, const std::string &client_name); /** * Set the default information in a directory znode struct diff --git a/zookeeper/source/zk_nn_client.cc b/zookeeper/source/zk_nn_client.cc index 41f4469a..9cc5c2b2 100644 --- a/zookeeper/source/zk_nn_client.cc +++ b/zookeeper/source/zk_nn_client.cc @@ -800,7 +800,8 @@ ZkNnClient::GetFileInfoResponse ZkNnClient::get_info( * Create a node in zookeeper corresponding to a file */ bool ZkNnClient::create_file_znode(const std::string &path, - FileZNode *znode_data) { + FileZNode *znode_data, + const std::string &client_name) { int error_code; if (!file_exists(path)) { LOG(INFO) << "[create_file_znode] Creating file znode at " << path; @@ -837,6 +838,19 @@ bool ZkNnClient::create_file_znode(const std::string &path, << error_code; return false; } + // grant the client the lease + if (!zk->create(LeaseZookeeperPath(path) + '/' + + client_name, ZKWrapper::EMPTY_VECTOR, + error_code, false)) { + LOG(ERROR) << "[create_file_znode] ZK create lease client path failed" + << error_code; + return false; + } + // set the timestamp for the client that created the file + RenewLeaseRequestProto req; + req.set_clientname(client_name); + RenewLeaseResponseProto res; + renew_lease(req, res); } return true; } @@ -1237,7 +1251,7 @@ ZkNnClient::CreateResponse ZkNnClient::create_file( } // if we failed, then do not set any status - if (!create_file_znode(path, &znode_data)) + if (!create_file_znode(path, &znode_data, request.clientname())) return CreateResponse::FailedCreateZnode; HdfsFileStatusProto *status = response.mutable_fs(); From 72f9b8108f7297669c3862c88ecbd160a2015df5 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 28 Nov 2017 00:03:18 -0600 Subject: [PATCH 2/6] Add test. --- test/namenode/CreationTest.cc | 21 +++++++++++++++++++++ zookeeper/source/zk_nn_client.cc | 5 +++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/test/namenode/CreationTest.cc b/test/namenode/CreationTest.cc index 75acb1db..974fd58a 100644 --- a/test/namenode/CreationTest.cc +++ b/test/namenode/CreationTest.cc @@ -144,3 +144,24 @@ TEST_F(NamenodeTest, creationPerformance) { } } +TEST_F(NamenodeTest, createLease) { + hadoop::hdfs::CreateRequestProto create_req = + getCreateRequestProto("basic_file"); + create_req.set_clientname("test_client1"); + hadoop::hdfs::CreateResponseProto create_resp; + ASSERT_EQ(client->create_file(create_req, create_resp), + zkclient::ZkNnClient::CreateResponse::Ok); + + hadoop::hdfs::HdfsFileStatusProto file_status = create_resp.fs(); + ASSERT_EQ(file_status.filetype(), + hadoop::hdfs::HdfsFileStatusProto::IS_FILE); + ASSERT_TRUE(client->file_exists("basic_file")); + + hadoop::hdfs::RecoverLeaseRequestProto recover_req; + recover_req.set_clientname("test_client2"); + recover_req.set_src("basic_file"); + hadoop::hdfs::RecoverLeaseResponseProto recover_res; + client->recover_lease(recover_req, recover_res); + ASSERT_FALSE(recover_res.result()); +} + diff --git a/zookeeper/source/zk_nn_client.cc b/zookeeper/source/zk_nn_client.cc index 9cc5c2b2..2cb78e0c 100644 --- a/zookeeper/source/zk_nn_client.cc +++ b/zookeeper/source/zk_nn_client.cc @@ -1372,6 +1372,7 @@ ZkNnClient::MkdirResponse ZkNnClient::mkdir(MkdirsRequestProto &request, ZkNnClient::MkdirResponse ZkNnClient::mkdir_helper(const std::string &path, bool create_parent) { LOG(INFO) << "[mkdir_helper] mkdir_helper called with input " << path; + std::string empty_string; if (create_parent) { std::vector split_path; boost::split(split_path, path, boost::is_any_of("/")); @@ -1391,7 +1392,7 @@ ZkNnClient::MkdirResponse ZkNnClient::mkdir_helper(const std::string &path, not_exist = true; FileZNode znode_data; set_mkdir_znode(&znode_data); - if (!create_file_znode(p_path, &znode_data)) { + if (!create_file_znode(p_path, &znode_data, empty_string)) { // TODO(2016) unroll the created directories return MkdirResponse::FailedZnodeCreation; } @@ -1400,7 +1401,7 @@ ZkNnClient::MkdirResponse ZkNnClient::mkdir_helper(const std::string &path, } else { FileZNode znode_data; set_mkdir_znode(&znode_data); - return create_file_znode(path, &znode_data) ? + return create_file_znode(path, &znode_data, empty_string) ? MkdirResponse::Ok : MkdirResponse::FailedZnodeCreation; } return MkdirResponse::Ok; From 842e030f565799d0a0a3c2950e92e7b400540eff Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 28 Nov 2017 10:26:09 -0600 Subject: [PATCH 3/6] fix lint errors. --- test/namenode/CreationTest.cc | 6 +++--- zookeeper/include/zk_nn_client.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/namenode/CreationTest.cc b/test/namenode/CreationTest.cc index 974fd58a..ba91f570 100644 --- a/test/namenode/CreationTest.cc +++ b/test/namenode/CreationTest.cc @@ -146,7 +146,7 @@ TEST_F(NamenodeTest, creationPerformance) { TEST_F(NamenodeTest, createLease) { hadoop::hdfs::CreateRequestProto create_req = - getCreateRequestProto("basic_file"); + getCreateRequestProto("basic_file_1"); create_req.set_clientname("test_client1"); hadoop::hdfs::CreateResponseProto create_resp; ASSERT_EQ(client->create_file(create_req, create_resp), @@ -155,11 +155,11 @@ TEST_F(NamenodeTest, createLease) { hadoop::hdfs::HdfsFileStatusProto file_status = create_resp.fs(); ASSERT_EQ(file_status.filetype(), hadoop::hdfs::HdfsFileStatusProto::IS_FILE); - ASSERT_TRUE(client->file_exists("basic_file")); + ASSERT_TRUE(client->file_exists("basic_file_1")); hadoop::hdfs::RecoverLeaseRequestProto recover_req; recover_req.set_clientname("test_client2"); - recover_req.set_src("basic_file"); + recover_req.set_src("basic_file_1"); hadoop::hdfs::RecoverLeaseResponseProto recover_res; client->recover_lease(recover_req, recover_res); ASSERT_FALSE(recover_res.result()); diff --git a/zookeeper/include/zk_nn_client.h b/zookeeper/include/zk_nn_client.h index 77a7210e..62505d58 100644 --- a/zookeeper/include/zk_nn_client.h +++ b/zookeeper/include/zk_nn_client.h @@ -613,7 +613,8 @@ class ZkNnClient : public ZkClientCommon { * Crate a znode corresponding to a file of "filetype", with path "path", with * znode data contained in "znode_data" */ - bool create_file_znode(const std::string &path, FileZNode *znode_data, const std::string &client_name); + bool create_file_znode(const std::string &path, FileZNode *znode_data, + const std::string &client_name); /** * Set the default information in a directory znode struct From 423b1de21bd46f1c6d6e004d907286f3c0015172 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 28 Nov 2017 11:47:17 -0600 Subject: [PATCH 4/6] Fix error. --- zookeeper/source/zk_nn_client.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zookeeper/source/zk_nn_client.cc b/zookeeper/source/zk_nn_client.cc index 2cb78e0c..9b2f32b6 100644 --- a/zookeeper/source/zk_nn_client.cc +++ b/zookeeper/source/zk_nn_client.cc @@ -404,7 +404,7 @@ void ZkNnClient::recover_lease(RecoverLeaseRequestProto &req, res.set_result(false); return; } - if (children.size() > 0) { + if (children.size() > 1) { for (std::string child : children) { LOG(ERROR) << "[recover_lease] Child: " + child; } From db29b0d9d186a1fc31ef639c70b414ab52d100aa Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 28 Nov 2017 12:53:29 -0600 Subject: [PATCH 5/6] Recover lease return true if current client is holding the lease. --- zookeeper/source/zk_nn_client.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zookeeper/source/zk_nn_client.cc b/zookeeper/source/zk_nn_client.cc index 9b2f32b6..0a683bfd 100644 --- a/zookeeper/source/zk_nn_client.cc +++ b/zookeeper/source/zk_nn_client.cc @@ -420,6 +420,10 @@ void ZkNnClient::recover_lease(RecoverLeaseRequestProto &req, return; } std::string lease_holder_client = children[0]; + if (lease_holder_client == client_name) { + res.set_result(true); + return; + } if (lease_expired(lease_holder_client)) { // TODO(elfyingying): start the lease recovery process // that closes the file for the From e73729cb3f6be3056d38d22e8cfb29358e03e594 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 28 Nov 2017 13:32:57 -0600 Subject: [PATCH 6/6] fix error. --- test/namenode/LeaseTest.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/namenode/LeaseTest.cc b/test/namenode/LeaseTest.cc index 4c6ddce3..dd8b6b8f 100644 --- a/test/namenode/LeaseTest.cc +++ b/test/namenode/LeaseTest.cc @@ -40,6 +40,7 @@ TEST_F(NamenodeTest, recoverLeaseCorrectnessTest) { // Create the file hadoop::hdfs::CreateRequestProto create_req = getCreateRequestProto("test_filename"); + create_req.set_clientname("test_client"); hadoop::hdfs::CreateResponseProto create_resp; ASSERT_EQ(client->create_file(create_req, create_resp), zkclient::ZkNnClient::CreateResponse::Ok);