diff --git a/src/base/status.h b/src/base/status.h index 4a4eb867724..6ce56f380af 100644 --- a/src/base/status.h +++ b/src/base/status.h @@ -19,6 +19,8 @@ #include +#include "absl/strings/str_cat.h" + #include "base/slice.h" #include "version.h" // NOLINT @@ -185,6 +187,7 @@ struct Status { inline bool OK() const { return code == ReturnCode::kOk; } inline const std::string& GetMsg() const { return msg; } inline int GetCode() const { return code; } + inline std::string ToString() const { return absl::StrCat("ReturnCode[", code, "]", msg); } int code; std::string msg; }; diff --git a/src/client/ns_client.h b/src/client/ns_client.h index f069ccce2d3..e48aaac955f 100644 --- a/src/client/ns_client.h +++ b/src/client/ns_client.h @@ -98,7 +98,7 @@ class NsClient : public Client { nameserver::ShowOPStatusResponse* response); base::Status ShowOPStatus(uint64_t op_id, ::openmldb::nameserver::ShowOPStatusResponse* response); - + // TODO bool CancelOP(uint64_t op_id, std::string& msg); // NOLINT bool AddTableField(const std::string& table_name, const ::openmldb::common::ColumnDesc& column_desc, @@ -143,13 +143,13 @@ class NsClient : public Client { const ::openmldb::nameserver::TableInfo& table_info, const ::openmldb::nameserver::ZoneInfo& zone_info, std::string& msg); // NOLINT - + // TODO bool AddReplica(const std::string& name, const std::set& pid_set, const std::string& endpoint, std::string& msg); // NOLINT bool AddReplicaNS(const std::string& name, const std::vector& endpoint_vec, uint32_t pid, const ::openmldb::nameserver::ZoneInfo& zone_info, const ::openmldb::api::TaskInfo& task_info); - + // TODO bool DelReplica(const std::string& name, const std::set& pid_set, const std::string& endpoint, std::string& msg); // NOLINT @@ -158,20 +158,20 @@ class NsClient : public Client { bool ConfGet(const std::string& key, std::map& conf_map, // NOLINT std::string& msg); // NOLINT - + // TODO bool ChangeLeader(const std::string& name, uint32_t pid, std::string& candidate_leader, // NOLINT std::string& msg); // NOLINT bool OfflineEndpoint(const std::string& endpoint, uint32_t concurrency, std::string& msg); // NOLINT - + // TODO bool Migrate(const std::string& src_endpoint, const std::string& name, const std::set& pid_set, const std::string& des_endpoint, std::string& msg); // NOLINT bool RecoverEndpoint(const std::string& endpoint, bool need_restore, uint32_t concurrency, std::string& msg); // NOLINT - + // TODO get msg bool RecoverTable(const std::string& name, uint32_t pid, const std::string& endpoint, std::string& msg); // NOLINT bool ConnectZK(std::string& msg); // NOLINT @@ -184,7 +184,7 @@ class NsClient : public Client { bool GetTablePartition(const std::string& name, uint32_t pid, ::openmldb::nameserver::TablePartition& table_partition, // NOLINT std::string& msg); // NOLINT - + // TODO bool UpdateTableAliveStatus(const std::string& endpoint, std::string& name, // NOLINT uint32_t pid, bool is_alive, diff --git a/src/client/tablet_client.cc b/src/client/tablet_client.cc index 9357b23e29a..2de831718c1 100644 --- a/src/client/tablet_client.cc +++ b/src/client/tablet_client.cc @@ -314,12 +314,13 @@ bool TabletClient::SendSnapshot(uint32_t tid, uint32_t remote_tid, uint32_t pid, return false; } -bool TabletClient::LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, uint32_t seg_cnt) { +base::Status TabletClient::LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, + uint32_t seg_cnt) { return LoadTable(name, id, pid, ttl, false, seg_cnt); } -bool TabletClient::LoadTable(const std::string& name, uint32_t tid, uint32_t pid, uint64_t ttl, bool leader, - uint32_t seg_cnt, std::shared_ptr task_info) { +base::Status TabletClient::LoadTable(const std::string& name, uint32_t tid, uint32_t pid, uint64_t ttl, bool leader, + uint32_t seg_cnt, std::shared_ptr task_info) { ::openmldb::api::TableMeta table_meta; table_meta.set_name(name); table_meta.set_tid(tid); @@ -330,10 +331,11 @@ bool TabletClient::LoadTable(const std::string& name, uint32_t tid, uint32_t pid } else { table_meta.set_mode(::openmldb::api::TableMode::kTableFollower); } - return LoadTable(table_meta, task_info); + return LoadTableInternal(table_meta, task_info); } -bool TabletClient::LoadTable(const ::openmldb::api::TableMeta& table_meta, std::shared_ptr task_info) { +base::Status TabletClient::LoadTableInternal(const ::openmldb::api::TableMeta& table_meta, + std::shared_ptr task_info) { ::openmldb::api::LoadTableRequest request; ::openmldb::api::TableMeta* cur_table_meta = request.mutable_table_meta(); cur_table_meta->CopyFrom(table_meta); @@ -341,28 +343,21 @@ bool TabletClient::LoadTable(const ::openmldb::api::TableMeta& table_meta, std:: request.mutable_task_info()->CopyFrom(*task_info); } ::openmldb::api::GeneralResponse response; - bool ok = client_.SendRequest(&::openmldb::api::TabletServer_Stub::LoadTable, &request, &response, - FLAGS_request_timeout_ms, 1); - if (ok && response.code() == 0) { - return true; + auto st = client_.SendRequestSt(&::openmldb::api::TabletServer_Stub::LoadTable, &request, &response, + FLAGS_request_timeout_ms, 1); + if (st.OK()) { + return {response.code(), response.msg()}; } - return false; + return st; } -bool TabletClient::LoadTable(uint32_t tid, uint32_t pid, std::string* msg) { - ::openmldb::api::LoadTableRequest request; - ::openmldb::api::TableMeta* table_meta = request.mutable_table_meta(); - table_meta->set_tid(tid); - table_meta->set_pid(pid); - table_meta->set_mode(::openmldb::api::TableMode::kTableLeader); - ::openmldb::api::GeneralResponse response; - bool ok = client_.SendRequest(&::openmldb::api::TabletServer_Stub::LoadTable, &request, &response, - FLAGS_request_timeout_ms, 1); - msg->swap(*response.mutable_msg()); - if (ok && response.code() == 0) { - return true; +bool TabletClient::LoadTable(const ::openmldb::api::TableMeta& table_meta, std::shared_ptr task_info) { + auto st = LoadTableInternal(table_meta, task_info); + // can't return msg, log here + if (!st.OK()) { + LOG(WARNING) << st.ToString(); } - return false; + return st.OK(); } bool TabletClient::ChangeRole(uint32_t tid, uint32_t pid, bool leader, uint64_t term) { diff --git a/src/client/tablet_client.h b/src/client/tablet_client.h index f955040a157..f6ff72e4cfa 100644 --- a/src/client/tablet_client.h +++ b/src/client/tablet_client.h @@ -135,15 +135,14 @@ class TabletClient : public Client { bool RecoverSnapshot(uint32_t tid, uint32_t pid, std::shared_ptr task_info = std::shared_ptr()); - bool LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, uint32_t seg_cnt); + base::Status LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, uint32_t seg_cnt); - bool LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, bool leader, uint32_t seg_cnt, + base::Status LoadTable(const std::string& name, uint32_t id, uint32_t pid, uint64_t ttl, bool leader, uint32_t seg_cnt, std::shared_ptr task_info = std::shared_ptr()); + // for ns WrapTaskFun, must return bool bool LoadTable(const ::openmldb::api::TableMeta& table_meta, std::shared_ptr task_info); - bool LoadTable(uint32_t tid, uint32_t pid, std::string* msg); - bool ChangeRole(uint32_t tid, uint32_t pid, bool leader, uint64_t term); bool ChangeRole(uint32_t tid, uint32_t pid, bool leader, const std::vector& endpoints, uint64_t term, @@ -166,7 +165,7 @@ class TabletClient : public Client { bool GetManifest(uint32_t tid, uint32_t pid, ::openmldb::common::StorageMode storage_mode, ::openmldb::api::Manifest& manifest); // NOLINT - + // TODO get msg bool GetTableStatus(::openmldb::api::GetTableStatusResponse& response); // NOLINT bool GetTableStatus(uint32_t tid, uint32_t pid, ::openmldb::api::TableStatus& table_status); // NOLINT @@ -175,7 +174,7 @@ class TabletClient : public Client { bool FollowOfNoOne(uint32_t tid, uint32_t pid, uint64_t term, uint64_t& offset); // NOLINT - + // TODO get msg bool GetTableFollower(uint32_t tid, uint32_t pid, uint64_t& offset, // NOLINT std::map& info_map, // NOLINT @@ -267,6 +266,9 @@ class TabletClient : public Client { bool GetAndFlushDeployStats(::openmldb::api::DeployStatsResponse* res); + private: + base::Status LoadTableInternal(const ::openmldb::api::TableMeta& table_meta, std::shared_ptr task_info); + private: ::openmldb::RpcClient<::openmldb::api::TabletServer_Stub> client_; }; diff --git a/src/cmd/openmldb.cc b/src/cmd/openmldb.cc index 3cf22b2df6d..9df5d86cb60 100644 --- a/src/cmd/openmldb.cc +++ b/src/cmd/openmldb.cc @@ -3227,12 +3227,13 @@ void HandleClientLoadTable(const std::vector parts, ::openmldb::cli return; } } - bool ok = client->LoadTable(parts[1], boost::lexical_cast(parts[2]), + // TODO(): get status msg + auto st = client->LoadTable(parts[1], boost::lexical_cast(parts[2]), boost::lexical_cast(parts[3]), ttl, is_leader, seg_cnt); - if (ok) { + if (st.OK()) { std::cout << "LoadTable ok" << std::endl; } else { - std::cout << "Fail to LoadTable" << std::endl; + std::cout << "Fail to LoadTable: " << st.ToString() << std::endl; } } catch (boost::bad_lexical_cast& e) { std::cout << "Bad LoadTable format" << std::endl; diff --git a/tools/openmldb_ops.py b/tools/openmldb_ops.py index c7ae0663b52..a72891fe65b 100644 --- a/tools/openmldb_ops.py +++ b/tools/openmldb_ops.py @@ -614,8 +614,9 @@ def PrettyPrint(data, header = None): sys.exit() executor = Executor(options.openmldb_bin_path, options.zk_cluster, options.zk_root_path) - if not executor.Connect().OK(): - log.error("connect OpenMLDB failed") + st = executor.Connect() + if not st.OK(): + log.error("connect OpenMLDB failed, {}".format(st.GetMsg())) sys.exit() if options.cmd in manage_ops: status, auto_failover = executor.GetAutofailover() diff --git a/tools/tool.py b/tools/tool.py index e64b172b49b..8e042b477b5 100644 --- a/tools/tool.py +++ b/tools/tool.py @@ -84,7 +84,7 @@ def Connect(self): cmd.append("--cmd=showns") status, output = self.RunWithRetuncode(cmd) if not status.OK() or status.GetMsg().find("zk client init failed") != -1: - return Status(-1, "get ns failed"), None + return Status(-1, "get ns failed") result = self.ParseResult(output) for record in result: if record[2] == "leader": @@ -97,7 +97,7 @@ def Connect(self): cmd.append("--cmd=showtablet") status, output = self.RunWithRetuncode(cmd) if not status.OK(): - return Status(-1, "get tablet failed"), None + return Status(-1, "get tablet failed") result = self.ParseResult(output) for record in result: if record[1] != '-': @@ -118,12 +118,14 @@ def RunWithRetuncode(self, command, useshell = USE_SHELL, env = os.environ): try: + log.info(" ".join(command)) p = subprocess.Popen(command, stdout = subprocess.PIPE, stderr = subprocess.PIPE, shell = useshell, universal_newlines = universal_newlines, env = env) - output = p.stdout.read() - p.wait() - errout = p.stderr.read() - p.stdout.close() - p.stderr.close() + output, errout = p.communicate() + # TODO(hw): the print from ns/tablet client are not standard, print it for debug + if output != "": + log.info(output) + if errout != "": + log.info(errout) if "error msg" in output: return Status(-1, output), output return Status(p.returncode, errout), output @@ -166,7 +168,7 @@ def GetAutofailover(self): return status, None if output.find("true") != -1: return Status(), True - return Status(), False; + return Status(), False def SetAutofailover(self, value): cmd = list(self.ns_base_cmd)